public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Microblaze: Type confusion in fprintf
@ 2019-09-12 13:33 Jonas Pfeil
  2019-09-20 22:50 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Jonas Pfeil @ 2019-09-12 13:33 UTC (permalink / raw)
  To: gcc-patches

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
        {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Microblaze: Type confusion in fprintf
  2019-09-12 13:33 [PATCH] Microblaze: Type confusion in fprintf Jonas Pfeil
@ 2019-09-20 22:50 ` Jeff Law
  2019-09-22  0:52   ` Segher Boessenkool
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2019-09-20 22:50 UTC (permalink / raw)
  To: Jonas Pfeil, gcc-patches

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.

jeff

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Microblaze: Type confusion in fprintf
  2019-09-20 22:50 ` Jeff Law
@ 2019-09-22  0:52   ` Segher Boessenkool
  0 siblings, 0 replies; 3+ messages in thread
From: Segher Boessenkool @ 2019-09-22  0:52 UTC (permalink / raw)
  To: Jeff Law; +Cc: Jonas Pfeil, gcc-patches

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-22  0:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 13:33 [PATCH] Microblaze: Type confusion in fprintf Jonas Pfeil
2019-09-20 22:50 ` Jeff Law
2019-09-22  0:52   ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).