[Bug tree-optimization/65215] [5 Regression] Bswap load miscompilation

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65215

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #9)
> (In reply to Thomas Preud'homme from comment #5)
> > (In reply to Richard Biener from comment #4)
> > > (In reply to Jakub Jelinek from comment #1)
> > > > Created attachment 34879 [details]
> > > > gcc5-pr65215.patch
> > > > 
> > > > Untested fix.  There are still issues left, e.g. I can't understand the
> > > > "bswap &&" part in
> > > >       if (bswap
> > > >           && align < GET_MODE_ALIGNMENT (TYPE_MODE (load_type))
> > > >           && SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align))
> > > >         return false;
> > > > Don't you use the new MEM_REF even for the !bswap (aka nop) case?  So, I
> > > > don't see how it would be safe to generate that.
> > > > And the testsuite coverage of this is definitely suboptimal, from endianity
> > > > POV, bitfields etc.
> > > 
> > > I suggested that change (remove bswap &&) multiple times, but it got lost
> > > appearantly.  (I even remember applying that change myself!?)

And what I changed was

Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c        (revision 212067)
+++ tree-ssa-math-opts.c        (revision 212068)
@@ -2179,7 +2179,9 @@ bswap_replace (gimple cur_stmt, gimple_s
       unsigned align;

       align = get_object_alignment (src);
-      if (bswap && SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align))
+      if (bswap
+         && align < GET_MODE_ALIGNMENT (TYPE_MODE (load_type))
+         && SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align))
        return false;

       gsi_move_before (&gsi, &gsi_ins);

> > 
> > I remember the contrary as this was the reason PR61320 was investigated in
> > the first place. This idea is that if it's unaligned the expander will take
> > care of this and that they would be at least as efficient as what the user
> > code was doing to avoid doing an unaligned load.
> 
> Ah indeed.  The idea is that the expander should be able to produce the
> most optimal and canonical form for an unaligned load on those targets,
> especially for say loading a 4-byte value from a 2-byte aligned source
> where the source had 4 individual char loads.
> 
> > I'm happy to revisit that position though.
> 
> No - I just stand corrected.
> 
> > Best regards,
> > 
> > Thomas