[PATCH] D29092: PR31729: [GVNHoist] Don't hoist unsafe scalars at -Oz

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

The test can probably be further simplified by removing unnecessary information:

- datalayout, sourcefilename, modulename
- TBAA metadata
- `; preds` comments
- the "all good" string

...



================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:510
     // where they are partially needed.
-    if (OptForMinSize)
+    if (OptForMinSize && isSafeToSpeculativelyExecute(I))
       return true;
----------------
hiraditya wrote:
> majnemer wrote:
> > Shouldn't we unconditionally do `isSafeToSpeculativelyExecute`?
> GVNHoist already has facilities to check the dependencies, safety etc. which is more efficient than calling isSafeToSpeculativelyExecute all the time for each instruction.
Sounds like at least an `assert(isSafeToSpeculativelyExecute());` is a good idea.


https://reviews.llvm.org/D29092