[Instcombine] -instcombine-verify-known-bits failure.

[JonPsson1]

tc_instcomb_knownbits.tar.gz

opt -mtriple=systemz-unknown -mcpu=z15 -O1 tc_instcomb_knownbits.bc -instcombine-verify-known-bits -o /dev/null

Mismatched known bits for %rem.i = srem i32 %conv.i, -1 in f
computeKnownBits(): 0000000000000000000000000000000?
SimplifyDemandedBits(): ????????????????????????????????

#8 0x000002aa01d6825c llvm::InstCombinerImpl::SimplifyDemandedInstructionBits

@RKSimon @jf-botto @nikic

[RKSimon]

CC @artagnon who is looking at something related on #109121

[nikic]

I fixed the immediate problem in 5ef02a3.

It would make sense to also use KnownBits::srem() here, but I noticed that the implementation (

llvm-project/llvm/lib/Support/KnownBits.cpp

Line 1061 in 5ef02a3

if (RHS.isConstant() && RHS.getConstant().isPowerOf2()) {

) only covers positive power of 2, while demanded bits simplification takes the absolute value (

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

Line 864 in 5ef02a3

APInt RA = Rem->abs();

).

[artagnon]

It would make sense to also use KnownBits::srem() here, but I noticed that the implementation (

llvm-project/llvm/lib/Support/KnownBits.cpp

Line 1061 in 5ef02a3

if (RHS.isConstant() && RHS.getConstant().isPowerOf2()) {

) only covers positive power of 2, while demanded bits simplification takes the absolute value (

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

Line 864 in 5ef02a3

APInt RA = Rem->abs();

).

I think the abs() doesn't make sense, since srem with a negative RHS is flipped into one with a positive RHS, changing LHS as appropriate. I tried exercising the abs() with different tests, with the same result: the change seems to be an NFC.

[nikic]

@artagnon Good point. Do you want to submit a PR to switch that code to KnownBits::srem?

[artagnon]

@artagnon Good point. Do you want to submit a PR to switch that code to KnownBits::srem?

On it. I'm currently working through the reasoning.