[PATCH] D125750: [InstCombine] fold fake floating point vector extract to shift+trunc.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan added a comment.

In D125750#3525445 <https://reviews.llvm.org/D125750#3525445>, @spatel wrote:

> Is there a motivating codegen example for this transform? Or some other IR transform that will fire as a result of this transform?
>
> In the case with a shift, we have an extra IR instruction, so this would be a rare fold that increases instruction count. Maybe that's justifiable just because we want to keep the symmetry with the other patterns, but it would be better to show some kind of win from this patch.

Mostly, it would be much cheaper if we use scalar shift + cast rather than bitcast + vector extractelemt, even the former might cause one more instruction in LLVMIR. For example in RISCV, the former one woule be lower to 3 scalar instructions, but the latter one would  firstly move from GPR to vector register and then use 2 vector instruction to extract the element, it is truely much more expensive, even without counting the vector configuration instruction that should be insert for using vector instructions.



================
Comment at: llvm/test/Transforms/InstCombine/extractelement.ll:512
+
+; negative test - only support the width <= 32
+
----------------
spatel wrote:
> This comment doesn't look accurate. The data layout for both RUN lines says i64 is legal, so that's why we allow converting i64 in the test above. And there's no exception for a 1-element vector. We probably should have a test like this to confirm:
>   define double @bitcast_fp64vec_index0(i64 %x) {
>     %v = bitcast i64 %x to <1 x double>
>     %r = extractelement <1 x double> %v, i8 0
>     ret double %r
>   }
> 
> Also make sure there's no infinite loop potential with that kind of pattern...I just noticed an odd behavior for it in D125951.
> 
> Please pre-commit the baseline tests, so it's easier to see what is or is not changing. Add a RUN with a legal i128 to confirm this is behaving as expected?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125750