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


Ping?

On Sep 27, 2013, at 11:21 AM, Mike Stump <mikestump@comcast.net> wrote:
> 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]