[llvm] r238447 - AsmPrinter: Suppress warnings on GCC from r238362, NFC

[David Blaikie]

On Thu, May 28, 2015 at 11:35 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Thu, May 28, 2015 at 2:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Thu, May 28, 2015 at 11:09 AM, Duncan P. N. Exon Smith
> > <dexonsmith at apple.com> wrote:
> >>
> >> Author: dexonsmith
> >> Date: Thu May 28 13:09:13 2015
> >> New Revision: 238447
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=238447&view=rev
> >> Log:
> >> AsmPrinter: Suppress warnings on GCC from r238362, NFC
> >>
> >> GCC seems to have some overzealous warnings about strict aliasing.
> >
> >
> > If the warning is wrong, can we just disable it rather than contorting
> the
> > code to suppress it? (especially with the risk that warnings like this in
> > GCC tend to be powered by their optimizer & thus might see through this &
> > start warning again)
>
> There were questions as to whether the warning was or was not a false
> positive which should be answered before turning off the warning over
> it.


According to C++ I believe the warning is correct, but I don't believe
there's any compliant way to do this. The correct thing you can do is use
placement new on the char buffer, then always use the resulting pointer -
but that requires extra storage for a pointer that's bit identical to the a
pointer to the char array and we're using this to save the space.


> That being said, I don't see a whole lot of strict aliasing
> warnings happening in general. If GCC's optimizer changes and starts
> warning on this again, we should probably take more time to understand
> why.
>

It looks like practically all uses (in Optional, DenseMap, SmallVector,
etc) of AlignedStorage, etc, are aliasing violations and would trigger this
warning if/when it's a bit more aggressive, etc.

I don't think there's much for it but to accept that we're not going to
conform to this part of the C++ standard any time soon.

- David


>
> ~Aaron
>
> >
> >>
> >> Rafael reports that this patch suppresses them on GCC 4.9, and I'm
> >> hoping this will work for GCC 4.7 as well.  I'll watch [1] and iterate
> >> if necessary.
> >>
> >> [1]: http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/8597
> >>
> >> Modified:
> >>     llvm/trunk/include/llvm/CodeGen/DIE.h
> >>
> >> Modified: llvm/trunk/include/llvm/CodeGen/DIE.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/DIE.h?rev=238447&r1=238446&r2=238447&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/CodeGen/DIE.h (original)
> >> +++ llvm/trunk/include/llvm/CodeGen/DIE.h Thu May 28 13:09:13 2015
> >> @@ -340,11 +340,11 @@ private:
> >>      new (reinterpret_cast<void *>(Val.buffer)) T(V);
> >>    }
> >>
> >> -  template <class T> T &get() { return *reinterpret_cast<T
> >> *>(Val.buffer); }
> >> -  template <class T> const T &get() const {
> >> -    return *reinterpret_cast<const T *>(Val.buffer);
> >> +  template <class T> T *get() { return reinterpret_cast<T
> *>(Val.buffer);
> >> }
> >> +  template <class T> const T *get() const {
> >> +    return reinterpret_cast<const T *>(Val.buffer);
> >>    }
> >> -  template <class T> void destruct() { get<T>().~T(); }
> >> +  template <class T> void destruct() { get<T>()->~T(); }
> >>
> >>    /// Destroy the underlying value.
> >>    ///
> >> @@ -378,11 +378,11 @@ private:
> >>        return;
> >>  #define HANDLE_DIEVALUE_SMALL(T)
> >> \
> >>    case is##T:
> >> \
> >> -    construct<DIE##T>(X.get<DIE##T>());
> >> \
> >> +    construct<DIE##T>(*X.get<DIE##T>());
> >> \
> >>      return;
> >>  #define HANDLE_DIEVALUE_LARGE(T)
> >> \
> >>    case is##T:
> >> \
> >> -    construct<const DIE##T *>(X.get<const DIE##T *>());
> >> \
> >> +    construct<const DIE##T *>(*X.get<const DIE##T *>());
> >> \
> >>      return;
> >>  #include "llvm/CodeGen/DIEValue.def"
> >>      }
> >> @@ -425,12 +425,12 @@ public:
> >>  #define HANDLE_DIEVALUE_SMALL(T)
> >> \
> >>    const DIE##T &getDIE##T() const {
> >> \
> >>      assert(getType() == is##T && "Expected " #T);
> >> \
> >> -    return get<DIE##T>();
> >> \
> >> +    return *get<DIE##T>();
> >> \
> >>    }
> >>  #define HANDLE_DIEVALUE_LARGE(T)
> >> \
> >>    const DIE##T &getDIE##T() const {
> >> \
> >>      assert(getType() == is##T && "Expected " #T);
> >> \
> >> -    return *get<const DIE##T *>();
> >> \
> >> +    return **get<const DIE##T *>();
> >> \
> >>    }
> >>  #include "llvm/CodeGen/DIEValue.def"
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/dbe87d31/attachment.html>