Re: [PATCH 01/10] Fix LRA bug

[Jeff Law <law@redhat.com>]

On 12/12/18 4:52 AM, Andrew Stubbs wrote:
> 
> [This is new patch not included in the previously posted patch sets.]
> 
> This patch fixes an ICE building libgfortran/random.c.
> 
> The problem was an adddi3 instruction that had an eliminable frame pointer.
> GCN adddi3 includes a match_scratch, which LRA substitutes with a REG, and
> checks if it can be converted back to a scratch afterwards.  In the meantime,
> the add was converted to a move, meaning that the instruction pattern
> completely changed, thus causing a segfault when the instruction is revisited
> in restore_scratches.
I'm torn here.  There's undocumented rules about this kind of stuff,
largely inherited from the days of "reload".  But I don't actually
recall the specifics of any of those rules.

I don't think there's any inherently wrong with what you've done in LRA.
 My worry is there's other places where this kind of changing the
structure of the underlying insn is going to cause you problems if your
patterns aren't following those undocumented rules.




> 
> 2018-12-12  Andrew Stubbs  <ams@codesourcery.com>
> 
> 	gcc/
> 	* gcc/lra-int.h (lra_register_new_scratch_op): Add third parameter.
> 	* gcc/lra-remat.c (update_scratch_ops): Pass icode to
> 	lra_register_new_scratch_op.
> 	* gcc/lra.c (struct sloc): Add icode field.
> 	(lra_register_new_scratch_op): Add icode parameter.
> 	Use icode to skip insns that have changed beyond recognition.
OK.  But be aware we may have to revisit and look more closely what what
you're doing in your port if we stumble over more problems with reload
changing the structure of your insns and causing problems in the process.

jeff