Zdenek Dvorak - Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_ins (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert

• From: Zdenek Dvorak
• To: Steven Bosscher
• Cc: gcc-patches at gcc dot gnu dot org
• Date: Mon, 26 Feb 2007 00:43:03 +0100
• Subject: Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
• References: <200702250134.02861.steven@gcc.gnu.org> <20070225184955.GA13143@atrey.karlin.mff.cuni.cz> <571f6b510702251457h1a41099aw79b1223356c1ffc6@mail.gmail.com> <20070225231043.GA472@atrey.karlin.mff.cuni.cz> <571f6b510702251536h57560c1bgc8d7c8750d864a6a@mail.gmail.com>

Hello,

On 2/25/07, Zdenek Dvorak rakdver@atrey.karlin.mff.cuni.cz wrote:

I do not dare to approve this; introducing code that relies on "blackbox" understanding of what some functions do seems a bit dangerous to me.

I don't understand this concern.

This is stage1. If something breaks, I can fix it.

I am more worried about the case when nothing breaks now, and in four years, someone will be really unhappy about having to decipher this hack.

Well thanks for bringing this up, because you're exactly describing the predicament I'm in here! :-)

See your own mail from even more than four years ago:

http://gcc.gnu.org/ml/gcc-patches/2002-05/msg01168.html

You added BB_SUPERBLOCK for some reason that even Honza didn't understand. You apparently were not completely sure yourself whether it was needed. Unfortunately you did not add a test case for the failure you observed.

well, everyone learns :-(

Now I'm the one having trouble deciphering why you added this hack (IMHO the real hack in gcc is that BB_SUPERBLOCK exists) in the first place. I assume you probably would have just updated the CFG in place if the proper infrastructure for that had been in place at the time.

Unless you fix the problem properly now, somebody else will have to do it later,

Yup. Wish you had done that back in 2002!

OK, teasing you :-P

hmm... no, you should be deadly serious. Yes, I should have fixed or at least documented it better back then.

I would love to say that I will fix the problem for you, and perhaps I will, if you are willing to wait for about two weeks (before I get rid of some other work that pilled on me just now).

I have no problem with waiting, but I can also fix the problem if I have a test case (I have a hackish patch, in fact). All I really want to see is whether any test suite failures show up somewhere, so that we have this code covered in the test suite.

To be honest, I don't believe we'll have any test suite failures, or other problems. But right now I see no other way to make sure, than by just trying. When something breaks after all, we can revert the patch for the moment and I can work on a proper fix (and of course I'd appreciate your help, if possible). But I can only fix a problem properly if I know that there is a problem to begin with.

I really would not propose this unusual way forward if I would see a better way out...

If you need that as a prerequisite for some other patches, I guess we might go this way temporarily; although long-term, somebody (and it seems like it will have to be myself :-) have to go through the code and find out why we are not seeing any problems.

Zdenek

• Follow-Ups:
  • Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Steven Bosscher
• References:
  • [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Steven Bosscher
  • Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Zdenek Dvorak
  • Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Steven Bosscher
  • Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Zdenek Dvorak
  • Re: [PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
    * From: Steven Bosscher