Wrong result for _BitInt bitwise & testcase

[mikaelholmen]

llvm commit: 6effab9
Reproducer:

clang bbi-100697.c -O2 -std=c23
./a.out

Result:

Fail!

This starts happening with 1833d41
Before that commit we got

Pass!

which is the expected result.

I originally reported this in #107461 (comment) but @alexey-bataev did some analysis in #107461 (comment) suggesting that the error is somewhere in X86 Codegen rather than in SLP vectorizer.

As I wrote in the original report the input was originally part of a much larger test so I've reduced it a lot but unfortunately it's still quite large (and ugly).

bbi-100697.c.gz

[mikaelholmen]

Slightly reduced C input in bbi-100697_2.c but it's still not very nice.

bbi-100697_2.c.gz

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: Mikael Holmén (mikaelholmen)

llvm commit: 6effab9 Reproducer: ``` clang bbi-100697.c -O2 -std=c23 ./a.out ``` Result: ``` Fail! ``` This starts happening with https://github.com/llvm/llvm-project/commit/1833d418a04123916c1dbeb0c41c8bc7d06b779b Before that commit we got ``` Pass! ``` which is the expected result.

I originally reported this in #107461 (comment) but @alexey-bataev did some analysis in #107461 (comment) suggesting that the error is somewhere in X86 Codegen rather than in SLP vectorizer.

As I wrote in the original report the input was originally part of a much larger test so I've reduced it a lot but unfortunately it's still quite large (and ugly).

bbi-100697.c.gz

[llvmbot]

@llvm/issue-subscribers-bug

Author: Mikael Holmén (mikaelholmen)

llvm commit: 6effab9 Reproducer: ``` clang bbi-100697.c -O2 -std=c23 ./a.out ``` Result: ``` Fail! ``` This starts happening with https://github.com/llvm/llvm-project/commit/1833d418a04123916c1dbeb0c41c8bc7d06b779b Before that commit we got ``` Pass! ``` which is the expected result.

I originally reported this in #107461 (comment) but @alexey-bataev did some analysis in #107461 (comment) suggesting that the error is somewhere in X86 Codegen rather than in SLP vectorizer.

As I wrote in the original report the input was originally part of a much larger test so I've reduced it a lot but unfortunately it's still quite large (and ugly).

bbi-100697.c.gz

[mikaelholmen]

Slightly more reduced C input in bbi-100697_3.c and somewhat cleaned up so it's a bit easier to understand what the C code is actually testing. I really have troubles getting further than this though.

bbi-100697_3.c.gz

[nikic]

Here's a reduction with disabled DAGCombine:

; RUN: llc -debug-counter=dagcombine=0 < %s
define i64 @test() {
    %x = bitcast i64 u0xffffffff00000001 to i64
    %t = trunc i64 %x to i32
    %a = and i32 %t, 1
    %e = zext i32 %a to i64
    ret i64 %e
}

Compiles to:

test:                                   # @test
        movabs  rax, -4294967295
        ret

What happens here is that the zext selects to subreg_to_reg because it's the result of an and which is going to zero the upper half anyway. But then shrinkAndImmediate will optimize away the and:

llvm-project/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

Lines 4834 to 4839 in 3a01b46

	// Check if the mask is -1. In that case, this is an unnecessary instruction
	// that escaped earlier analysis.
	if (NegMaskVal.isAllOnes()) {
	ReplaceNode(And, And0.getNode());
	return true;
	}

So we end up not doing the required clearing of the top bits.

@RKSimon Any thoughts on the best way to fix this? The def32 mechanism looks pretty fragile to me...

[RKSimon]

Its supposed to have been caught by this:

llvm-project/llvm/lib/Target/X86/X86InstrExtension.td

Lines 214 to 222 in 26fd693

	// The preferred way to do 32-bit-to-64-bit zero extension on x86-64 is to use a
	// SUBREG_TO_REG to utilize implicit zero-extension, however this isn't possible
	// when the 32-bit value is defined by a truncate or is copied from something
	// where the high bits aren't necessarily all zero. In such cases, we fall back
	// to these explicit zext instructions.
	def : Pat<(i64 (zext GR32:$src)),
	(SUBREG_TO_REG (i64 0), (MOV32rr GR32:$src), sub_32bit)>;
	def : Pat<(i64 (zextloadi64i32 addr:$src)),
	(SUBREG_TO_REG (i64 0), (MOV32rm addr:$src), sub_32bit)>;

[nikic]

@RKSimon The problem is that we match the zext first, at which point the and still exists.

[RKSimon]

Yes, still just working this out in my head:

Optimized legalized selection DAG: %bb.0 'test:'
SelectionDAG has 10 nodes:
    t0: ch,glue = EntryToken
        t3: i32 = truncate OpaqueConstant:i64<-4294967295>
      t5: i32 = and t3, Constant:i32<1>
    t6: i64 = zero_extend t5
  t9: ch,glue = CopyToReg t0, Register:i64 $rax, t6
  t10: ch = X86ISD::RET_GLUE t9, TargetConstant:i32<0>, Register:i64 $rax, t9:1


Selected selection DAG: %bb.0 'test:'
SelectionDAG has 11 nodes:
    t0: ch,glue = EntryToken
        t2: i64 = MOV64ri TargetConstant:i64<-4294967295>
      t3: i32 = EXTRACT_SUBREG t2, TargetConstant:i32<6>
    t6: i64 = SUBREG_TO_REG TargetConstant:i64<0>, t3, TargetConstant:i32<6>
  t9: ch,glue = CopyToReg t0, Register:i64 $rax, t6
  t10: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t9, t9:1