[PATCH] D66132: [CodeGen] Add `isSpeculativeExecutionForbidden()` to `TargetTransformInfo`

Tue Aug 13 05:08:39 PDT 2019

anton-afanasyev added a comment.

In D66132#1626811 <https://reviews.llvm.org/D66132#1626811>, @lebedev.ri wrote:

> I feel like this is lacking words.
>  What problem is this addressing?
>  Is it correctness-one or performance?
>  The current hook feels a little bit weird.
>  Does it mean to completely ban speculative execution?
>  If then there is a lot more places where it should be used, and not using it there may be surprising. 
>  Or does it only mean to ban PRE? But then, what was the original problem?
>  Or should it be per-MI hook?
> etc

The problem is discussed here: https://bugs.llvm.org/show_bug.cgi?id=42405
Per-MI hook already exists -- it is `hasSideEffect()`. But for particular targets setting this to `true` is undesirable, this breaks existing optimizations (though it could really have side effect, like `div` for aarch64. This change means to ban PRE at all, yes.
@igorb could add something here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66132