diff mbox

[1/2,combine] Try REG_EQUAL for nonzero_bits

Message ID 20150212083449.GM4274@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Feb. 12, 2015, 8:34 a.m. UTC 
On Tue, Feb 10, 2015 at 11:03:57PM -0700, Jeff Law wrote:
> On 02/09/15 19:19, Thomas Preud'homme wrote:
> >>From: Andrew Pinski [mailto:pinskia@gmail.com]
> >>Sent: Tuesday, February 10, 2015 9:57 AM
> >
> >>>+#ifdef SHORT_IMMEDIATES_SIGN_EXTEND
> >>>+/* If MODE has a precision lower than PREC and SRC is a non-negative
> >>constant
> >>>+   that would appear negative in MODE, sign-extend SRC for use in
> >>nonzero_bits
> >>>+   because some machines (maybe most) will actually do the sign-
> >>extension and
> >>>+   this is the conservative approach.
> >>>+
> >>>+   ??? For 2.5, try to tighten up the MD files in this regard instead of
> >>this
> >>>+   kludge.  */
> >>
> >>I don't know if this has been mentioned and even though you are just
> >>copying a comment from below but would it make sense to look fixing
> >>what the comment says we should look at after GCC 2.5 (which was over
> >>20 years ago)? Or maybe just remove the comment if it no longer
> >>applies.
> >
> >Actually this bit seems unnecessary as there is already some logic in
> >nonzero_bits1 for the CONST_INT case. So I guess the code can be
> >removed and the comment be moved there at the very least but
> >I'd prefer people from one of the affected target to test it.

I can tell you that the following doesn't trigger on an
--enable-targets=all,go powerpc64-linux bootstrap.  (Ada not built due
to lack of gnat on the machine I used.)  So for powerpc it looks like
the combine SHORT_IMMEDIATES_SIGN_EXTEND code can disappear.  The
rtlanal.c occurrence *is* executed.

Comments

Thomas Preud'homme Feb. 13, 2015, 9:39 a.m. UTC | #1 
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Thursday, February 12, 2015 4:35 PM
> > >
> > >Actually this bit seems unnecessary as there is already some logic in
> > >nonzero_bits1 for the CONST_INT case. So I guess the code can be
> > >removed and the comment be moved there at the very least but
> > >I'd prefer people from one of the affected target to test it.
> 
> I can tell you that the following doesn't trigger on an
> --enable-targets=all,go powerpc64-linux bootstrap.  (Ada not built due
> to lack of gnat on the machine I used.)  So for powerpc it looks like
> the combine SHORT_IMMEDIATES_SIGN_EXTEND code can disappear.
> The
> rtlanal.c occurrence *is* executed.

So I build a lm32-elf cross-compiler with and without the code guarded by
this macro (both in combine.c and rtlanal.c) and then compiled as much as
I could of gcc (make -I -k) and compared the object files.

diff tells me that there is no difference whatsoever. If you want me to do
tests on other programs or for other target please let me know.

Best regards,

Thomas
diff mbox

Patch

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	(revision 220611)
+++ gcc/combine.c	(working copy)
@@ -1742,7 +1742,7 @@  set_nonzero_bits_and_sign_copies (rtx x, const_rtx
 	      && CONST_INT_P (src)
 	      && INTVAL (src) > 0
 	      && val_signbit_known_set_p (GET_MODE (x), INTVAL (src)))
-	    src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (GET_MODE (x)));
+	    gcc_unreachable ();
 #endif
 
 	  /* Don't call nonzero_bits if it cannot change anything.  */
@@ -9802,7 +9802,7 @@  reg_nonzero_bits_for_combine (const_rtx x, machine
 	  && CONST_INT_P (tem)
 	  && INTVAL (tem) > 0
 	  && val_signbit_known_set_p (GET_MODE (x), INTVAL (tem)))
-	tem = GEN_INT (INTVAL (tem) | ~GET_MODE_MASK (GET_MODE (x)));
+	gcc_unreachable ();
 #endif
       return tem;
     }