[InstCombine] Failure to convert vector fp comparisons that can be represented as integers

[RKSimon]

https://rust.godbolt.org/z/dfWjnToef

We will simplify a comparison of scalar floats that can be represented as integers to compare the integers directly, but we don't do this for vectors:

define i32 @cmpf(i32 %x) {
  %and = and i32 %x, 3
  %conv = sitofp i32 %and to float
  %cmp = fcmp oeq float %conv, 3.000000e+00
  %sext = sext i1 %cmp to i32
  ret i32 %sext
}

define <8 x i32> @vcmpf(<8 x i32> %x) {
  %and = and <8 x i32> %x, <i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3>
  %conv = sitofp <8 x i32> %and to <8 x float>
  %cmp = fcmp oeq <8 x float> %conv, <float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00>
  %sext = sext <8 x i1> %cmp to <8 x i32>
  ret <8 x i32> %sext
}

opt -O3

define i32 @cmpf(i32 %x) {
  %and = and i32 %x, 3
  %cmp = icmp eq i32 %and, 3
  %sext = sext i1 %cmp to i32
  ret i32 %sext
}

define <8 x i32> @vcmpf(<8 x i32> %x) {
  %and = and <8 x i32> %x, <i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3>
  %conv = sitofp <8 x i32> %and to <8 x float>
  %cmp = fcmp oeq <8 x float> %conv, <float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00>
  %sext = sext <8 x i1> %cmp to <8 x i32>
  ret <8 x i32> %sext
}

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. In the comments of the issue, request for it to be assigned to you.
2. Fix the issue locally.
3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a Git commit.
5. Run git clang-format HEAD~1 to format your changes.
6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Simon Pilgrim (RKSimon)

https://rust.godbolt.org/z/dfWjnToef

We will simplify a comparison of scalar floats that can be represented as integers to compare the integers directly, but we don't do this for vectors:

define i32 @<!-- -->cmpf(i32 %x) {
  %and = and i32 %x, 3
  %conv = sitofp i32 %and to float
  %cmp = fcmp oeq float %conv, 3.000000e+00
  %sext = sext i1 %cmp to i32
  ret i32 %sext
}

define &lt;8 x i32&gt; @<!-- -->vcmpf(&lt;8 x i32&gt; %x) {
  %and = and &lt;8 x i32&gt; %x, &lt;i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3&gt;
  %conv = sitofp &lt;8 x i32&gt; %and to &lt;8 x float&gt;
  %cmp = fcmp oeq &lt;8 x float&gt; %conv, &lt;float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00&gt;
  %sext = sext &lt;8 x i1&gt; %cmp to &lt;8 x i32&gt;
  ret &lt;8 x i32&gt; %sext
}

opt -O3

define i32 @<!-- -->cmpf(i32 %x) {
  %and = and i32 %x, 3
  %cmp = icmp eq i32 %and, 3
  %sext = sext i1 %cmp to i32
  ret i32 %sext
}

define &lt;8 x i32&gt; @<!-- -->vcmpf(&lt;8 x i32&gt; %x) {
  %and = and &lt;8 x i32&gt; %x, &lt;i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3&gt;
  %conv = sitofp &lt;8 x i32&gt; %and to &lt;8 x float&gt;
  %cmp = fcmp oeq &lt;8 x float&gt; %conv, &lt;float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00, float 3.000000e+00&gt;
  %sext = sext &lt;8 x i1&gt; %cmp to &lt;8 x i32&gt;
  ret &lt;8 x i32&gt; %sext
}

[SahilPatidar]

@dtcxzyw,
I would like to work on this one.

[SahilPatidar]

@RKSimon and @dtcxzyw,

I have been reviewing the foldFCmpIntToFPConst function in InstCombineCompares.cpp and have noticed a potential issue regarding vector of floats. The current implementation only checks for ConstantFP, which means it would return nullptr for vector of floats.

I am planning to make some changes to address this issue and related code. Before proceeding, I wanted to confirm if my understanding is correct and if there is a specific procedure I should follow for making these changes.
Thank you for your time and assistance.

[RKSimon]

Just using Constant::getSplatValue(false) would safely solve this for splat vectors (with no undefs)

[SahilPatidar]

@RKSimon,
I'm seeking clarification on a line in the foldFCmpIntToFPConst function in LLVM:

IntegerType *IntTy = cast<IntegerType>(LHSI->getOperand(0)->getType());

This line is casting the type of the first operand of LHSI to IntegerType. However, the type of the operand is actually a vector type <8 x i32>. Could you provide guidance on how to properly handle this situation?

[RKSimon]

AFAICT you can tweak it to work with scalar or vector integer types by changing it to:

auto *IntTy = cast<IntegerType>(LHSI->getOperand(0)->getType()->getScalarType());

Type::getScalarType will just pass through scalar types, but will return the element type for vectors

[RKSimon]

Although the more I look at it - I don't think we need the IntegerType type at all - IntTy->getBitWidth() is just the same as IntTy->getScalarSizeInBits() below.

auto *IntTy = LHSI->getOperand(0)->getType();
unsigned InputSize = IntTy->getScalarSizeInBits();
...
APSInt RHSCvt(InputSize , LHSUnsigned);