On Wed, Dec 16, 2020 at 04:36:37PM +0100, Jakub Jelinek wrote: > On Wed, Dec 16, 2020 at 09:28:56AM -0600, Segher Boessenkool wrote: > > This used to not work, as mentioned in the original patch submission: > > https://patchwork.ozlabs.org/project/gcc/patch/52f14532eb742ac8d878a185a46a88da7b0326eb.1442588483.git.seg...@kernel.crashing.org/ > > I wonder what changed? > > There is no testcase in that thread I could find, on which it would ICE.
It was just a bootstrap+regression check, so no new testcase was needed. I don't remember what target, but powerpc (32+64, BE) probably. > The testcase in the patch (which I've excended today compared to what > the patch had yesterday) has the cases of a single as well as multiple > edges to the ideal prologue bb in the cold partition, yet I can't reproduce > the ICE. > As for what changed, I remember fixing PR87475 which is related to > redirection of crossing jumps. But that is for cfglayout mode? Shrink-wrap has to do everything in cfgrtl mode unfortunately, because of the partitioning stuff. Sounds like a good cancidate otherwise. > > So, the situation is there are multiple incoming edges to where we want > > to place the prologue. Because we have to have one single edge to put > > the prologue on (that is how the infrastructure we have works, it is not > > anything fundamental), we create a new block that just jumps to the > > block that needs the prologue, use that new edge for where we place the > > prologue, and then redirect all edges to the first block to the new > > block. (We cannot in all case simply fall through.) > > I believe this case is covered in the testcase. Yeah... It is never certaint what it will look like in the generated machine code, but yup, probably. In either case, if regstrap now works with this on all targets, this is great :-) I just wish I knew what changed. > > > In the former case, they > > > are usually conditional jumps that patch_jump_insn can handle just fine, > > > after all, they were previously crossing and will be crossing after > > > the redirection too, just to a different label. And in the powerpc64 > > > case, it is a simple_jump instead that again seems to be handled by > > > patch_jump_insn just fine. > > > > So what changed in the last five years that makes redirecting > > EDGE_CROSSING jumps now always work? Does that also apply to > > EDGE_COMPLEX, btw? Knowing this would make at least me a lot less > > nervous about this patch :-) > > I think EDGE_COMPLEX generally can't be redirected, it is ok to punt on > those. Yeah, it is the kitchen sink "do not touch this" flag :-) Thanks again, Segher
