Followup for reg_equiv_invariant patch: Fix PR39871

[Bernd Schmidt <bernds at codesourcery dot com>]

For flag_pic, reload currently disables the reg_equiv_invariant
machinary, which leads to poor code generation in PR39871 (a variant of
PR42500, 42502 and 40615 which were fixed by my earlier patch).

The problematic if statement looks like this:

	  if (! function_invariant_p (x)
	      || ! flag_pic
	      /* A function invariant is often CONSTANT_P but may
		 include a register.  We promise to only pass
		 CONSTANT_P objects to LEGITIMATE_PIC_OPERAND_P.  */
	      || (CONSTANT_P (x)
		  && LEGITIMATE_PIC_OPERAND_P (x)))

It has a bit of history.  Originally, we only supported REG_EQUIV for
constants, and it looked like this:

         if (note
#ifdef LEGITIMATE_PIC_OPERAND_P
             && (! CONSTANT_P (XEXP (note, 0)) || ! flag_pic
                 || LEGITIMATE_PIC_OPERAND_P (XEXP (note, 0)))
 #endif
             )

This simply avoids using constants that aren't LEGITIMATE_PIC_OPERAND_P.
 Then, Joern added the machinery that allows us to process equivalences
for things that are function_invariant_p, and I think this is where the
mistake happened:

 #ifdef LEGITIMATE_PIC_OPERAND_P
-             && (! CONSTANT_P (XEXP (note, 0)) || ! flag_pic
+             && (! function_invariant_p (XEXP (note, 0))
+                 || ! flag_pic
                  || LEGITIMATE_PIC_OPERAND_P (XEXP (note, 0)))
 #endif

This looks like an error - the patch replaces several occurrences of
CONSTANT_P with function_invariant_p in order to enable the extra
optimization, but here I think the test should simply have been left
alone; we deal with the various cases of equivalences inside this if
statement.

This also introduced a new bug: later on, someone else noticed that this
code could now pass non-constant objects to LEGITIMATE_PIC_OPERAND_P,
and added the extra CONSTANT_P check around that test, which gives us
the current version of the code.

The patch below simply restores this to essentially the original form.
Regression tested on arm-none-linux-gnueabi with my usual set of
multilibs (also bootstrapped and regression tested on i686-linux last
week IIRC, will retest before checking in).

Joern, any comments - do you recall any reason why this change would
have been intentional?


Bernd

Attachment: pic-equiv.diff
Description: Text document