This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Need help with PR71976 combine.c::get_last_value returns a wrong result


Segher Boessenkool schrieb:
On Tue, Jul 26, 2016 at 03:38:18PM +0200, Georg-Johann Lay wrote:
@@ -13206,6 +13206,13 @@ get_last_value (const_rtx x)
      && DF_INSN_LUID (rsp->last_set) >= subst_low_luid)
    return 0;

+ /* If the lookup is for a hard register make sure that value contains at
least
+     as many bits as x does.  */
+
+  if (regno < FIRST_PSEUDO_REGISTER
+      && GET_MODE_PRECISION (rsp->last_set_mode) < GET_MODE_PRECISION
(GET_MODE (x)))
+    return 0;
+
  /* If the value has all its registers valid, return it.  */
if (get_last_value_validate (&value, rsp->last_set, rsp->last_set_label,
  0))
    return value;
That might be a bit harsh.
In what respect?

It disables all kinds of optimisations that work now.

First things first: is the information recorded correct?
I think yes, but care must be taken what may be concluded from that information. From a set of 8 bits you cannot draw conclusion about all 64 bits; this should be obvious enough :-)

:-)

In the above case rsp[regno] holds only information for 1 sub-byte. In order to get the complete DImode story we would have to get the info for all sub-parts and then put them together...

Yes, and the rsp stuff does not do that.

I am testing the following patch.  Thanks,


Segher


diff --git a/gcc/combine.c b/gcc/combine.c
index 77e0d2b..dec6226 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -9977,6 +9977,9 @@ reg_num_sign_bit_copies_for_combine (const_rtx x, machine_mode mode,
       return NULL;
     }
+ if (GET_MODE_PRECISION (rsp->last_set_mode) != GET_MODE_PRECISION (mode))
+    return NULL;
+
   tem = get_last_value (x);
   if (tem != 0)
     return tem;

But the problem is inside get_last_value. You'd have to add such a check at /all/ call sites of that function. Using a value for a hard reg that's been set in a smaller mode than the mode of get_last_value is just wrong.

For example combine tracks bits which are known to be zero, and if get_last_value is used in a similar situation, then the conclusion about zero-bits is also wrong.

Would be interesting to patch get_last_value so that it issues some diagnostic if

if (regno < FIRST_PSEUDO_REGISTER
    && (GET_MODE_PRECISION (rsp->last_set_mode)
        < GET_MODE_PRECISION (GET_MODE (x))))

and then run the testsuite against that compiler. I would expect that the condition doesn't even trigger on x86.

Johann


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]