This is the mail archive of the gcc-patches@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: Commit: MSP430: Pass -md on to assembler


On Sep 27, 2013, at 1:48 AM, nick clifton <nickc@redhat.com> wrote:
> OK by me, although I cannot approve that particular patch.

I know, the intent is for someone that can, to approve it.

> But I ran into a very strange problem.  With your PARTIAL_INT_MODE_NAME patch applied GCC started erroneously eliminating NULL function pointer checks!  This was particularly noticeable in libgcc/crtstuff.c where for example:

Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 202634)
+++ stor-layout.c	(working copy)
@@ -2821,7 +2821,7 @@ get_mode_bounds (enum machine_mode mode,
 		 enum machine_mode target_mode,
 		 rtx *mmin, rtx *mmax)
 {
-  unsigned size = GET_MODE_BITSIZE (mode);
+  unsigned size = GET_MODE_PRECISION (mode);
   unsigned HOST_WIDE_INT min_val, max_val;
 
   gcc_assert (size <= HOST_BITS_PER_WIDE_INT);

fixes this problem.  The problem is that we treat the maximum value of PSImode as -1, and then later we do:

          case EQ:
            /* x == y is always false for y out of range.  */
  =>        if (val < mmin || val > mmax)
B             return const0_rtx;
            break;

and the answer to the question is 0 > -1, is no, so the entire test is eliminated as never able to be true.  After the fix, in your case, we get:

(gdb) p mmin
$72 = -524288
(gdb) p mmax
$73 = 524287

and the test becomes if (0 < -524288 || 0 > 524287), which is not true, then the test isn't eliminated as never true.

Here, we see the test that protects this code uses GET_MODE_PRECISION:

(gdb) macro expand HWI_COMPUTABLE_MODE_P (PSImode)
expands to: ((((enum mode_class) mode_class[PSImode]) == MODE_INT || ((enum mode_class) mode_class[PSIm\
ode]) == MODE_PARTIAL_INT) && mode_precision[PSImode] <= (8 * 8))

so, clearly, someone was thinking about GET_MODE_PRECISION being in range, not GET_MODE_BITSIZE.

Ok?  [ for the rtl people ]

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