[PATCH] D57871: Fix some cases where icmp (bitcast ([su]itofp X)), Y is misfolded

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4827-4828
 
+static unsigned getVectorNumElementsOrZero(Type *T)
+{
+	return T->isVectorTy() ? T->getVectorNumElements() : 0;
----------------
clang-format would have not put `{` on a new line.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:4953
 
   // Zero-equality and sign-bit checks are preserved through sitofp + bitcast.
   Value *X;
----------------
Adjust comment too?
`// Zero-equality and sign-bit checks are preserved through sitofp + bitcast, if bitcast does not change the number of elements.`


================
Comment at: test/Transforms/InstCombine/cast-int-icmp-eq-0.ll:658
+
+define <6 x i1> @i16_cast_cmp_sgt_int_m1_bitcast_vector_num_elements_sitofp(<3 x i16> %i) {
+; CHECK-LABEL: @i16_cast_cmp_sgt_int_m1_bitcast_vector_num_elements_sitofp(
----------------
Can you please also duplicated these cases with `bitcast` casting fp vector to an scalar integer?


================
Comment at: test/Transforms/InstCombine/cast-int-icmp-eq-0.ll:665-666
+;
+  %f = sitofp <3 x i16> %i to  <3 x float>
+  %b = bitcast <3 x float> %f to <6 x i16>
+  %cmp = icmp sgt <6 x i16> %b, <i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1>
----------------
craig.topper wrote:
> lebedev.ri wrote:
> > I agree that this currently crashes, but are you sure this is valid in the first place?
> > `%f = sitofp <3 x i16> %i to  <3 x float>` implies that `sizeof(float)` == `sizeof(i16)`
> > `%b = bitcast <3 x float> %f to <6 x i16>` implies that `sizeof(float)` == `2*sizeof(i16)`.
> > How can `bitcast` do that?
> > 
> > https://llvm.org/docs/LangRef.html#bitcast-to-instruction
> > > The bit sizes of value and the destination type, ty2, must be identical.
> > > It is always a no-op cast because no bits change with this conversion.
> > > The conversion is done as if the value had been stored to memory and read back as type ty2. 
> sizeof(float) is always 32 bits. Why does sitofp <3 x i16> %i to  <3 x float> imply sizeof(float) == sizeof(i16)? There's no requirement that the integer and fp width be equal for that.
> sizeof(float) is always 32 bits.

Aha. Haven't fully woken up yet.
Which means the `bitcast` is correct, and `sitofp` is "increasing the bit width".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57871