BUMP: AW: [PATCH] Microblaze: Type confusion in fprintf

["Jonas Pfeil" <jonas.pfeil@uni-ulm.de>]

Yes, it is a 32-bit host and 32-bit target.
HOST_WIDE_INT is long long (on ARM).
Also it says 4 lines above that the type of value_long is unsigned long, so
lets use the specifier for unsigned long.

Jonas

-----Ursprüngliche Nachricht-----
Von: Segher Boessenkool <segher@kernel.crashing.org> 
Gesendet: Sonntag, 22. September 2019 02:52
An: Jeff Law <law@redhat.com>
Cc: Jonas Pfeil <jonas.pfeil@uni-ulm.de>; gcc-patches@gcc.gnu.org
Betreff: Re: [PATCH] Microblaze: Type confusion in fprintf

On Fri, Sep 20, 2019 at 04:50:12PM -0600, Jeff Law wrote:
> On 9/12/19 7:33 AM, Jonas Pfeil wrote:
> > A type confusion leads to illegal (and nonsensical) assembly on 
> > certain host architectures (e.g. ARM), where HOST_WIDE_INT (64 bit) 
> > and unsigned long (32
> > bit) have different alignments. So this has printed an uninitialized 
> > register instead of the intended value. This fixes float constant 
> > initialization on an ARM->Microblaze cross compiler.
> > 
> > --- a/gcc/config/microblaze/microblaze.c
> > +++ b/gcc/config/microblaze/microblaze.c
> > @@ -2468,7 +2468,7 @@ print_operand (FILE * file, rtx op, int letter)
> >           unsigned long value_long;
> >           REAL_VALUE_TO_TARGET_SINGLE (*CONST_DOUBLE_REAL_VALUE (op),
> >                                        value_long);
> > -         fprintf (file, HOST_WIDE_INT_PRINT_HEX, value_long);
> > +         fprintf (file, "%#lx", value_long);
> >         }
> >        else
> >         {
> > 
> But shouldn't HOST_WIDE_INT_PRINT_HEX here result in the same thing?
> 
> >From hwint.h:
> 
> #ifndef HAVE_INTTYPES_H
> #if INT64_T_IS_LONG
> # define GCC_PRI64 HOST_LONG_FORMAT
> #else
> # define GCC_PRI64 HOST_LONG_LONG_FORMAT #endif
> 
> [ ... ]
> 
> #undef PRIx64
> #define PRIx64 GCC_PRI64 "x"
> [ ... ]
> 
> #define HOST_WIDE_INT_PRINT_HEX "%#" PRIx64
> 
> 
> If that's not giving us back %#lx, then it seems to me like something 
> is wrong elsewhere.

This is a 32-bit target though, so INT64_T_IS_LONG is false.


Segher