[PATCH] D75505: [InstCombine] Enhance cast-of-load->load-of-bitcast fold to handle multiple identical casts

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:571
+      // Note that if load is atomic, we might not be able to change load type.
+      if (LI.isAtomic() && !isSupportedAtomicType(DestTy))
+        return nullptr;
----------------
lebedev.ri wrote:
> jfb wrote:
> > lebedev.ri wrote:
> > > jfb wrote:
> > > > This seems like a pre-existing bug: you should bail on volatile as well. For example, if user code does a volatile load of int32 and then casts to float, we should probably try to emit an integer load (even if that's not guaranteed by the language).
> > > I can do that, but could you point me to the appropriate langref wording?
> > I don't think we're super clear on what we guarantee, but knowing people who use volatile: when they say "load an integer and cast it to a float" they expect to do an integer, followed by an integer->float register transfer. That's a reasonable expectation IMO.
> People also expect UB to be ignored by compiler.
> Unless it's spelled out in langref i don't believe i know why this should be changed.
That's a user-hostile approach to compiler optimizations. It's not a useful thing to optimize, and developers don't have a way to express this, so they'd have to turn to assembly. We could simply be nice and do what's trivial for us to do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75505/new/

https://reviews.llvm.org/D75505