[AArch64] Split bitmask immediate of bitwise AND operation

[jaykang10]

Summary

MOVi32imm + ANDWrr ==> ANDWri + ANDWri
MOVi64imm + ANDXrr ==> ANDXri + ANDXri

The mov pseudo instruction could be expanded to multiple mov instructions later. In this case, try to split the constant operand of mov instruction into two bitmask immediates. It makes only two AND instructions intead of multiple mov + and instructions.

Added a peephole optimization pass on MIR level to implement it.

[jaykang10]

In D109963#3008656, @dmgreen wrote:

I have tried to implement it on ISelDAG level. There are already patterns to fold and node. After this transformation on ISelDAGToDAG, I was able to see the patterns failed to match. If we can guarantee the pattern for this transformation is matched after matching other patterns related to and, it could be ok to implement it on ISelDAG level. Maybe, we could add AddedComplexity to the pattern for this transformation but I thought the CustomInserter is better than it because it guarantees all pattern matching is done.

Custom Inserters are mostly needed for instructions that expand to multiple basic blocks. As far as I understand, this seems to be different selection from and(X, C), which should fit in fine as an AArch64 tblgen pattern or with Dag2Dag select. But the AArch64 backend can be a bit complex in places. Was there something getting in the way of that? If so do you know what?

For example, there are patterns as below.

multiclass SIMDAcrossLanesUnsignedIntrinsic<string baseOpc,
                                            SDPatternOperator opNode>
    : SIMDAcrossLanesIntrinsic<baseOpc, opNode> {
// If there is a masking operation keeping only what has been actually
// generated, consume it.
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v8i8 V64:$Rn)), (i64 0)), (i64 0))), maski8_or_more)),
      (i32 (EXTRACT_SUBREG
        (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
          (!cast<Instruction>(!strconcat(baseOpc, "v8i8v")) V64:$Rn), bsub),
        ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v16i8 V128:$Rn)), (i64 0))),
            maski8_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v16i8v")) V128:$Rn), bsub),
          ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v4i16 V64:$Rn)), (i64 0)), (i64 0))), maski16_or_more)),
          (i32 (EXTRACT_SUBREG
            (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
              (!cast<Instruction>(!strconcat(baseOpc, "v4i16v")) V64:$Rn), hsub),
            ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v8i16 V128:$Rn)), (i64 0))),
            maski16_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v8i16v")) V128:$Rn), hsub),
          ssub))>;
}

As you can see, the patterns checks roughly (and (...), mask[8|16]_or_more) and it folds the and node. When I tried to split the bitmask immediate on ISelDAGToDAG level, I saw the cases in which above patterns does not work because the mask[8]16]_or_more constraint is failed.

As other case, on AArch64ISelDAGToDAG, '(or (and' patterns are folded by tryBitfieldInsertOp(). After splitting the bitmask immediate, I saw the cases in which the tryBitfieldInsertOp() is failed.

I have not checked all of regressions but there were more cases in which there are more instructions after splitting the bitmask immediate on ISelDAGToDAG level. In order to avoid it, I implemented the logic with CustomInserter.

[Wilco1]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2281 ↗

(On Diff #373207)

Is there no function to check it is a 16-bit immediate? If not, it would be best to add it.

2290 ↗

(On Diff #373207)

This won't correctly handle 32-bit immediates like 0x1234FFFF since the invert sets the top 32 bits.

2292 ↗

(On Diff #373207)

Should be * 8.

2308 ↗

(On Diff #373207)

These need to be 64 bits

[dmgreen]

For example, there are patterns as below.

multiclass SIMDAcrossLanesUnsignedIntrinsic<string baseOpc,
                                            SDPatternOperator opNode>
    : SIMDAcrossLanesIntrinsic<baseOpc, opNode> {
// If there is a masking operation keeping only what has been actually
// generated, consume it.
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v8i8 V64:$Rn)), (i64 0)), (i64 0))), maski8_or_more)),
      (i32 (EXTRACT_SUBREG
        (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
          (!cast<Instruction>(!strconcat(baseOpc, "v8i8v")) V64:$Rn), bsub),
        ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v16i8 V128:$Rn)), (i64 0))),
            maski8_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v16i8v")) V128:$Rn), bsub),
          ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v4i16 V64:$Rn)), (i64 0)), (i64 0))), maski16_or_more)),
          (i32 (EXTRACT_SUBREG
            (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
              (!cast<Instruction>(!strconcat(baseOpc, "v4i16v")) V64:$Rn), hsub),
            ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v8i16 V128:$Rn)), (i64 0))),
            maski16_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v8i16v")) V128:$Rn), hsub),
          ssub))>;
}

As you can see, the patterns checks roughly (and (...), mask[8|16]_or_more) and it folds the and node. When I tried to split the bitmask immediate on ISelDAGToDAG level, I saw the cases in which above patterns does not work because the mask[8]16]_or_more constraint is failed.

As other case, on AArch64ISelDAGToDAG, '(or (and' patterns are folded by tryBitfieldInsertOp(). After splitting the bitmask immediate, I saw the cases in which the tryBitfieldInsertOp() is failed.

I have not checked all of regressions but there were more cases in which there are more instructions after splitting the bitmask immediate on ISelDAGToDAG level. In order to avoid it, I implemented the logic with CustomInserter.

OK I see. So when adding the code to AArch64DAGToDAGISel::Select, it happened before the tablegen patterns and we do have tablegen patterns that conflict with it. I'm a little surprised to see "_or_more" here.

Could it be done as a tblgen pattern then? That way the larger pattern should win. As far as I understand it would be a case of getting the correct ImmLeaf with a couple of xforms.

[jaykang10]

In D109963#3009100, @dmgreen wrote:

For example, there are patterns as below.

multiclass SIMDAcrossLanesUnsignedIntrinsic<string baseOpc,
                                            SDPatternOperator opNode>
    : SIMDAcrossLanesIntrinsic<baseOpc, opNode> {
// If there is a masking operation keeping only what has been actually
// generated, consume it.
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v8i8 V64:$Rn)), (i64 0)), (i64 0))), maski8_or_more)),
      (i32 (EXTRACT_SUBREG
        (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
          (!cast<Instruction>(!strconcat(baseOpc, "v8i8v")) V64:$Rn), bsub),
        ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v16i8 V128:$Rn)), (i64 0))),
            maski8_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v16i8v")) V128:$Rn), bsub),
          ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (insert_subvector undef,
            (opNode (v4i16 V64:$Rn)), (i64 0)), (i64 0))), maski16_or_more)),
          (i32 (EXTRACT_SUBREG
            (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
              (!cast<Instruction>(!strconcat(baseOpc, "v4i16v")) V64:$Rn), hsub),
            ssub))>;
def : Pat<(i32 (and (i32 (vector_extract (opNode (v8i16 V128:$Rn)), (i64 0))),
            maski16_or_more)),
        (i32 (EXTRACT_SUBREG
          (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)),
            (!cast<Instruction>(!strconcat(baseOpc, "v8i16v")) V128:$Rn), hsub),
          ssub))>;
}

As you can see, the patterns checks roughly (and (...), mask[8|16]_or_more) and it folds the and node. When I tried to split the bitmask immediate on ISelDAGToDAG level, I saw the cases in which above patterns does not work because the mask[8]16]_or_more constraint is failed.

As other case, on AArch64ISelDAGToDAG, '(or (and' patterns are folded by tryBitfieldInsertOp(). After splitting the bitmask immediate, I saw the cases in which the tryBitfieldInsertOp() is failed.

I have not checked all of regressions but there were more cases in which there are more instructions after splitting the bitmask immediate on ISelDAGToDAG level. In order to avoid it, I implemented the logic with CustomInserter.

OK I see. So when adding the code to AArch64DAGToDAGISel::Select, it happened before the tablegen patterns and we do have tablegen patterns that conflict with it. I'm a little surprised to see "_or_more" here.

Could it be done as a tblgen pattern then? That way the larger pattern should win. As far as I understand it would be a case of getting the correct ImmLeaf with a couple of xforms.

As I mentioned on previous comment, the patterns for splitting the bitmask would need AddedComplexity to guarantee that the other patterns with and are already matched. I thought it could not be good for potential patterns with and to be added in the future.
From my personal opinion, MIR level could be suitable place to handle the bitmask immediate. At this moment, AArch64 target generates Pseudo MOV MIR with immediate which is big and expand it on AArch64ExpandPseudo. It could be good to generate Pseudo AND or logical MIR with the bitmask immediate and expand it like MOV. However, it could be big change...
Let me try to add the patterns with xform and AddedComplexity.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

2281 ↗

(On Diff #373207)

We can check it with isUInt<16>(Mask). Let me update it.

2290 ↗

(On Diff #373207)

You are right! Let me update it.

2292 ↗

(On Diff #373207)

Sorry, it is my mistake. Maybe, it could be 16 instead of 8?

2308 ↗

(On Diff #373207)

You are right! Let me update it.

[jaykang10]

Following the comment of @dmgreen, updated patch using TableGen patterns.

[efriedma]

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

8

Do we want to do something different if the operation is in a loop, so the immediate could be hoisted?

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

374

This looks wrong?

llvm/test/CodeGen/AArch64/bitfield-insert.ll

13

This isn't an improvement.

[dmgreen]

Nice job making the tablegen patterns work.

Is it possible to combine more of the 32 and 64bit logic? They share quite a bit in common. A few more tests sound useful too, for various edge cases.

llvm/lib/Target/AArch64/AArch64InstrInfo.td

1962

This can probably use AArch64_IMM::expandMOVImm? And Count the number of Instructions it produces. A similar "how many instructions will this immediate produce" was needed in https://reviews.llvm.org/D108871

[jaykang10]

In D109963#3011413, @dmgreen wrote:

Nice job making the tablegen patterns work.

Is it possible to combine more of the 32 and 64bit logic? They share quite a bit in common. A few more tests sound useful too, for various edge cases.

Yep, let me update patch with it.

llvm/lib/Target/AArch64/AArch64InstrInfo.td

1962

Yep, let me update code with it.

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

8

um... I do not think the split bitmask immediate affects MachineLICM pass to detect the invariant code.

Let me add a test case with loop.

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

374

um... I think it is correct.

mov x9, #31
movk x9, #48, lsl #32
==>
(48 << 32) | 31 = 31

and x8, x8, #0x3f
and x8, x8, #0xffffffffffffffdf
==>
0xffffffffffffffdf & 0x3f = 31

llvm/test/CodeGen/AArch64/bitfield-insert.ll

13

Let me update code to avoid it.

[jaykang10]

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

374

Sorry, I made a mistake. x register is 64 bit... Let me fix it.

[jaykang10]

Following comments, updated patch and fixed a bug.

[jaykang10]

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

374

I have fixed it with new patch.

[dmgreen]

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

133

I think Eli was referring to something like this: https://godbolt.org/z/YMYcqrroT
Which may or may be meaningfully worse, depending on vectorizations and constant hoisting.

[jaykang10]

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

133

Ah, let me have a look.

[jaykang10]

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

8

um... I understand it now...

If instruction has register operand and its definition is in loop, the instruction is not loop invariant in MachineLoop...

AND[W|X]ri has register operand rather than MOVi32imm...

um... Thanks @efriedma. Let me think about it more...

[jaykang10]

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

8

We could need to expand MOVi32imm on pseudo expansion with considering AND... or run a machine function pass which expands the MOVi32imm with AND after MachineLICM...
@efriedma @dmgreen How do you think about it?

[dmgreen]

I guess there are three options:

• Ignore it, because we believe it will not come up enough in practice enough or will be dealt with elsewhere (I'm not sure this is always true).
• Teach ISel lowering that the BB is a loop and pick different strategies based on it.
• Do it post-sel in a peephole optimization pass or machine combining pass, that knows about being in a loop or critical patch latencies.

For point 1, the question probably becomes will these cases always be vectorized/unrolled at high optimization levels, in which case it will either be a different (vector) instruction doing most of the work, or the constant would be hoisted by constant hoisting. And when optimizing for size the smaller codesize is probably OK.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

386 ↗

(On Diff #373902)

"Create"

[jaykang10]

Added a peephole optimization pass on MIR level to split bitmask immediate.

[jaykang10]

Updated comments

[dmgreen]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

52 ↗

(On Diff #374485)

Can this preserve CFG, which should help it preserve some analysis passes

66 ↗

(On Diff #374485)

Can this use regsize == sizeof(T), to simplify it a little

81 ↗

(On Diff #374485)

Does it matter if MI is loop invariant? As opposed to the MOV.

129 ↗

(On Diff #374485)

Should this transform be done with multiple uses on the mov?

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

This can move into the peephole optimization pass now, which might allow it to be simplified somewhat.

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

52 ↗

(On Diff #374485)

Yep, I think so. Let me add AU.setPreservesCFG.

66 ↗

(On Diff #374485)

Yep, let me update it.

81 ↗

(On Diff #374485)

Let's say MOV is in preheader and AND is in the loop as below.

preheader:
  %1:gpr32 = MOVi32imm 2098176
  %2:gpr32 =
...
loop:
... 
  %3:gpr32 = ANDWrr killed %2:gpr32, %1:gpr32

In above example, AND is loop invariant and we can split the constant of MOV.

preheader:
  %1:gpr32 = MOVi32imm 2098176
...
loop:
...
  %2:gpr32 = PHI 
  %3:gpr32 = ANDWrr killed %2:gpr32, %1:gpr32

In above example, the AND is not loop invariant and we can not split the constant of MOV because the first AND needs %2 and the AND is outside loop.

129 ↗

(On Diff #374485)

I thought there could be other opportunities with two bitmask immediate and other bitwise operations like OR. If possible, I would like to keep the MOV for the potential opportunities.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

This function could be used on other places. If possible, I would like to keep this check here conservatively.

[jaykang10]

Following comments from @dmgreen, updated patch.

[dmgreen]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

85 ↗

(On Diff #374522)

Consider renaming DefMI to MovMI, to clearly specify what it is expected to contain.

103 ↗

(On Diff #374522)

Typo in immediate

108 ↗

(On Diff #374522)

A different typo in immediate :)

129 ↗

(On Diff #374485)

Yeah that might well happen. Do you have any tests? Maybe in an unrolled loop?

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

What other places do you expect it to be used?

It seems at the moment, with the way these are called they will be re-calculating the same values repeatedly. That was unavoidable from tablegen patterns, but shouldn't be necessary from the new Peephole pass, hopefully.

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

85 ↗

(On Diff #374522)

Yep, let me update it.

103 ↗

(On Diff #374522)

Sorry, let me update it.

108 ↗

(On Diff #374522)

Sorry, again... Let me update it.

129 ↗

(On Diff #374485)

I do not have test cases yet. I could try to split the bitmask more later.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

You are right! Let me move it to the peephole pass.

[dmgreen]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #374485)

I just think it's worth having some tests to show this is working as intended. Maybe where the other uses are transformed and where they are not. (Ignore the unrolled loop comment - it wouldn't trigger in a loop).

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #374485)

Let me add a simple test in which and and or use same constant.

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #374485)

Sorry, I was wrong... Let's see below example.

define void @test10(i32* nocapture %x, i32* nocapture readonly %y, i32* nocapture %z) {
entry:
  %0 = load i32, i32* %y, align 4
  %and = and i32 %0, 2098176
  store i32 %and, i32* %x, align 4
  %1 = load i32, i32* %y, align 4
  %or = or i32 %1, 2098176
  store i32 %or, i32* %z, align 4
  ret void
}

After instruction selection, the MIR is as below.

; %4:gpr32 = MOVi32imm 2098176
; %5:gpr32 = ANDWrr killed %3:gpr32, %4:gpr32
; STRWui killed %5:gpr32, %0:gpr64common, 0 :: (store (s32) into %ir.x, !tbaa !8) 
; %6:gpr32 = LDRWui %1:gpr64common, 0 :: (load (s32) from %ir.y, !tbaa !8) 
; %7:gpr32 = ORRWrr killed %6:gpr32, %4:gpr32

We can see the and and or share the constant. The final assembly is as below.

	mov	w9, #1024
	movk	w9, #32, lsl #16
	and	w8, w8, #0x3ffc00
	and	w8, w8, #0xffe007ff
	str	w8, [x0]
	ldr	w8, [x1]
	orr	w8, w8, w9

If there are multiple uses of the constant, we should not split the constant because it could cause more instructions as above. I am sorry for mistake. Let me update code.

[jaykang10]

Following the comments from @dmgreen, updated patch.

[dmgreen]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

97 ↗

(On Diff #374805)

"because"

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

Was this going to be moved?

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

97 ↗

(On Diff #374805)

Sorry, let me update it.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h

342 ↗

(On Diff #374485)

Ah, sorry. I misunderstood what you want. Let me move this function to the peephole optimization pass.

[jaykang10]

Following comments of @dmgreen, updated patch.

[dmgreen]

My point in moving them was that they could be simplified, especially if inlined. As far as I can tell the two calls to splitAndBitmaskImm are recalculating values that were already computed in isValidAndSplitBitmaskImm. Can we not just calculate the values once?

[jaykang10]

In D109963#3024009, @dmgreen wrote:

My point in moving them was that they could be simplified, especially if inlined. As far as I can tell the two calls to splitAndBitmaskImm are recalculating values that were already computed in isValidAndSplitBitmaskImm. Can we not just calculate the values once?

Let me update it.

[jaykang10]

Following comment of @dmgreen, updated patch.

[dmgreen]

Thanks. LGTM

This kind of code can be incredibly easy to get wrong. Perhaps try running a bootstrap or something like it to flush out any issues.

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

146 ↗

(On Diff #375295)

This appears to be checked here and at the start of splitBitmaskImm.

[jaykang10]

In D109963#3026741, @dmgreen wrote:

Thanks. LGTM

This kind of code can be incredibly easy to get wrong. Perhaps try running a bootstrap or something like it to flush out any issues.

Thanks for review. Let me run bootstrap build and check some benchmarks.

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

146 ↗

(On Diff #375295)

Yep, let me remove this one.

[jaykang10]

Fixed a bug

• Same MI could be added to ToBeRemoved multiple time.
• Replaced SmallVector by SmallSetVector for ToBeRemoved

[jaykang10]

In D109963#3026759, @jaykang10 wrote:

In D109963#3026741, @dmgreen wrote:

Thanks. LGTM

This kind of code can be incredibly easy to get wrong. Perhaps try running a bootstrap or something like it to flush out any issues.

Thanks for review. Let me run bootstrap build and check some benchmarks.

I have checked bootstrap build and test-suite. After fixing a bug, they are passed. Let me push this patch.

[jaykang10]

Fixed a bug.

The destination register class of AND register is different with AND immediate one. In order to align the register class, the COPY MI is added to convert register class as below.

  %1:gpr32 = MOVi32imm 2098176
  %2:gpr32common = ANDWrr %0:gpr32, killed %1:gpr32
==>
  %5:gpr32sp = ANDWri %0:gpr32, 1419
  %6:gpr32 = COPY %5:gpr32sp
  %7:gpr32sp = ANDWri %6:gpr32, 725
  %2:gpr32common = COPY %7:gpr32sp

[haowei]

We are seeing clang crashes after this patch was relanded in https://reviews.llvm.org/rG73a196a11c0e6fe7bbf33055cc2c96ce3c61ff0d by @jaykang10 . Failed build task: https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.arm64-release-subbuild/b8834828820366166433/overview and reproducer: https://storage.cloud.google.com/fuchsia-build/builds/8834828820366166433/bcm-990d09.tar.gz Looks like an assertion error.

I locally revert your change and retest clang with this reproducer and the clang no longer crashes. Could you revert your change if it takes a while to fix it?

[saugustine]

Hi, I have tracked a crash down to this patch. This cases passes at f701505c45c7 and fails at this change. The target is aarch64 and the case is run at -O1.

Unfortunately, the test case is very large and I am trying to reduce it. But for now, here is the stack trace of the failure:

1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'util-e32393.cpp'.
4.	Running pass 'Live Variable Analysis' on function '@_ZN8colossus13ParseOpenModeENSt3__u17basic_string_viewIcNS0_11char_traitsIcEEEE'
 #0 0x0000000004d0cd9a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/Support/Unix/Signals.inc:565:11
 #1 0x0000000004d0cf6b PrintStackTraceSignalHandler(void*) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/Support/Unix/Signals.inc:632:1
 #2 0x0000000004d0b50b llvm::sys::RunSignalHandlers() /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/Support/Signals.cpp:96:5
 #3 0x0000000004d0d6e1 SignalHandler(int) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
 #4 0x00007f30c11dd140 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
 #5 0x00007f30c0c7cce1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #6 0x00007f30c0c66537 abort ./stdlib/abort.c:81:7
 #7 0x00007f30c0c6640f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8
 #8 0x00007f30c0c6640f _nl_load_domain ./intl/loadmsgcat.c:970:34
 #9 0x00007f30c0c75662 (/lib/x86_64-linux-gnu/libc.so.6+0x34662)
#10 0x00000000038aff71 llvm::LiveVariables::HandleVirtRegUse(llvm::Register, llvm::MachineBasicBlock*, llvm::MachineInstr&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:131:20
#11 0x00000000038b27fc llvm::LiveVariables::runOnInstr(llvm::MachineInstr&, llvm::SmallVectorImpl<unsigned int>&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:540:7
#12 0x00000000038b2c9d llvm::LiveVariables::runOnBlock(llvm::MachineBasicBlock*, unsigned int) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:572:25
#13 0x00000000038b331d llvm::LiveVariables::runOnMachineFunction(llvm::MachineFunction&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/CodeGen/LiveVariables.cpp:640:5
#14 0x00000000039676b7 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:72:8
#15 0x000000000404db6c llvm::FPPassManager::runOnFunction(llvm::Function&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1439:23
#16 0x0000000004052e95 llvm::FPPassManager::runOnModule(llvm::Module&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1485:16
#17 0x000000000404e534 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1554:23
#18 0x000000000404e058 llvm::legacy::PassManagerImpl::run(llvm::Module&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:542:16
#19 0x00000000040531a1 llvm::legacy::PassManager::run(llvm::Module&) /usr/local/google/home/saugustine/llvm-new/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1681:3
#20 0x00000000051cd032 (anonymous namespace)::EmitAssemblyHelper::EmitAssemblyWithNewPassManager(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/CodeGen/BackendUtil.cpp:1516:3
#21 0x00000000051ca597 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/CodeGen/BackendUtil.cpp:1675:5
#22 0x0000000006808fe0 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:334:7
#23 0x0000000008649013 clang::ParseAST(clang::Sema&, bool, bool) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/Parse/ParseAST.cpp:178:12
#24 0x0000000005caa882 clang::ASTFrontendAction::ExecuteAction() /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/Frontend/FrontendAction.cpp:1065:1
#25 0x00000000068047bb clang::CodeGenAction::ExecuteAction() /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:1072:5
#26 0x0000000005caa248 clang::FrontendAction::Execute() /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/Frontend/FrontendAction.cpp:960:7
#27 0x0000000005bdfb12 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:974:23
#28 0x0000000005e74716 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:278:8
#29 0x00000000014ea90d cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/tools/driver/cc1_main.cpp:246:13
#30 0x00000000014dd3d8 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/tools/driver/driver.cpp:317:5
#31 0x00000000014dc3f1 main /usr/local/google/home/saugustine/llvm-new/llvm-project/clang/tools/driver/driver.cpp:388:5
#32 0x00007f30c0c67d0a __libc_start_main ./csu/../csu/libc-start.c:308:16
#33 0x00000000014dbbea _start (/usr/local/google/home/saugustine/llvm-new/build/bin/clang+0x14dbbea)

[craig.topper]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

132 ↗

(On Diff #375557)

Do you need to check if the SUBREG_TO_REG has multiple uses?

[saugustine]

Since we have a reproducer, I have reverted this change with rGc07f7099690e.

[jaykang10]

In D109963#3029134, @saugustine wrote:

Since we have a reproducer, I have reverted this change with rGc07f7099690e.

Sorry for the error.

I have fixed it and I can see the reproducer from @haowei is passed. After checking tests and benchmarks again, let me try to push this patch again.

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

132 ↗

(On Diff #375557)

You are right! I need to check it as well as mov. Thanks @craig.topper!

[jaykang10]

Additionally, if possible, @saugustine can you file a bug with your test case to bugzilla please? It could be different failure.

[jaykang10]

Fixed bugs

• Check MOVimm before checking multiple uses
• Check SUBREG_TO_REG has multiple uses

[dmgreen]

I don't think this needs to create all the COPY's, if it calls constrainRegClass on the registers to ensure they do not contain the wrong set of registers.

[jaykang10]

In D109963#3029409, @dmgreen wrote:

I don't think this needs to create all the COPY's, if it calls constrainRegClass on the registers to ensure they do not contain the wrong set of registers.

You are right! We can constrain the register class to GPR[32|64]common. Thanks @dmgreen. Let me update it.

[jaykang10]

Following comment of @dmgreen, updated patch.

• Removed COPY MIs by constraining register classes.

[dmgreen]

Thanks. I think this LGTM, providing the issues reported now working OK.

[jaykang10]

In D109963#3029797, @dmgreen wrote:

Thanks. I think this LGTM, providing the issues reported now working OK.

Yep, I am checking test-suites with different optimization levels. Once it is ok, let me try to push this patch again.

[jaykang10]

@haowei @saugustine If possible, can you check your examples are ok with latest update of this patch please?

[haowei]

In D109963#3029824, @jaykang10 wrote:

@haowei @saugustine If possible, can you check your examples are ok with latest update of this patch please?

I no longer see this error in our build after applied your latest patch.

[jaykang10]

In D109963#3031089, @haowei wrote:

In D109963#3029824, @jaykang10 wrote:

@haowei @saugustine If possible, can you check your examples are ok with latest update of this patch please?

I no longer see this error in our build after applied your latest patch.

Thanks for checking it. Let me push this patch again.

[zatrazz]

This has caused a regression on stage2 buildbot [1], more specifically the test FAIL: Clang::p4-0x.cpp (but I think the other others might be related).

[1] https://lab.llvm.org/buildbot/#/builders/185/builds/543

[jaykang10]

In D109963#3042177, @zatrazz wrote:

This has caused a regression on stage2 buildbot [1], more specifically the test FAIL: Clang::p4-0x.cpp (but I think the other others might be related).

[1] https://lab.llvm.org/buildbot/#/builders/185/builds/543

um... I checked the bootstrap build before pushing this patch. Let me check it again.

[jaykang10]

In D109963#3042220, @jaykang10 wrote:

In D109963#3042177, @zatrazz wrote:

This has caused a regression on stage2 buildbot [1], more specifically the test FAIL: Clang::p4-0x.cpp (but I think the other others might be related).

[1] https://lab.llvm.org/buildbot/#/builders/185/builds/543

um... I checked the bootstrap build before pushing this patch. Let me check it again.

It looks like the test has -fsyntax-only command line option so I think clang patches have caused the error.

[DavidSpickett]

I've also reproduced it. Since check stage 1 passes my guess is that the clang built in stage 1 is emitting incorrect code.

[jaykang10]

In D109963#3042600, @DavidSpickett wrote:

I've also reproduced it. Since check stage 1 passes my guess is that the clang built in stage 1 is emitting incorrect code.

um... I checked it on AArch64 machine as below.

make stage2 -j32
./bin/llvm-lit ../clang/test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
llvm-lit: /home/jinkan01/Projects/llvm-project/llvm/utils/lit/lit/llvm/config.py:436: note: using clang: /home/jinkan01/Projects/llvm-project/build/bin/clang
-- Testing: 1 tests, 1 workers --
PASS: Clang :: CXX/over/over.match/over.match.funcs/p4-0x.cpp (1 of 1)

Testing Time: 0.22s
  Passed: 1

If possible, can you let me know how to reproduce it please? @DavidSpickett @zatrazz

[DavidSpickett]

If you have docker:

$ docker pull linaro/ci-arm64-tcwg-llvmbot-ubuntu:bionic
$ docker run -it --entrypoint=/bin/bash linaro/ci-arm64-tcwg-llvmbot-ubuntu:bionic
root@a9e83ff71f75:/# export CC=/usr/local/clang+llvm-12.0.0-aarch64-linux-gnu/bin/clang
root@a9e83ff71f75:/# export CXX=/usr/local/clang+llvm-12.0.0-aarch64-linux-gnu/bin/clang++
<follow commands from the buildbot page>

I suggest following https://lab.llvm.org/buildbot/#/builders/179/builds/1234 because it's the simplest. (not by much, it just doesn't use lld)

If you don't, then the things to note are that clang-12 is used for stage 1.

I'll check with the system gcc as well.

Edit: yes it does with gcc building stage 1.

[leonardchan]

I believe the re-land also caused a couple of tests to break for us in our 2-stage build:

Failed Tests (4):
  Clang :: CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
  Clang :: CodeGenCXX/catch-undef-behavior.cpp
  Clang :: Profile/def-assignop.cpp
  Clang :: SemaCXX/type-traits.cpp

https://luci-milo.appspot.com/ui/p/fuchsia/builders/prod/clang-linux-arm64/b8834634043817970513/overview

[jaykang10]

Sorry, let me try to reproduce the errors.

[jaykang10]

Fixed a bug

• For the 32 bit form of instruction, the upper 32 bits of the destination register are set to zero. If there is SUBREG_TO_REG, set the upper 32 bits of UImm to zero.

[jaykang10]

I am sorry for late. It took a time to find the problem and check the stage2 build again.

I was enable to reproduce the regressions with stage2 build. I have fixed them and I can see the regressions are gone.

@DavidSpickett @leonardchan @zatrazz If possible, can you check the regressions with latest update of this patch please?

If there are still regressions with the update, please let me know.

[jaykang10]

In D109963#3049312, @jaykang10 wrote:

I am sorry for late. It took a time to find the problem and check the stage2 build again.

I was enable to reproduce the regressions with stage2 build. I have fixed them and I can see the regressions are gone.

@DavidSpickett @leonardchan @zatrazz If possible, can you check the regressions with latest update of this patch please?

If there are still regressions with the update, please let me know.

Following https://lab.llvm.org/buildbot/#/builders/179/builds/1234, I can see the ninja check-all of stage2 build with the latest update of this patch is passed on AArch64 machine.

[DavidSpickett]

If there are still regressions with the update, please let me know.

Looks good on our side. I think you can reland now and see if anyone else still has failures with that.

[jaykang10]

In D109963#3050597, @DavidSpickett wrote:

If there are still regressions with the update, please let me know.

Looks good on our side. I think you can reland now and see if anyone else still has failures with that.

Thanks @DavidSpickett for checking it! Let me re-commit this patch.

[danielkiss]

This patch causes a compiler crash, the reduced reproducer is here> https://godbolt.org/z/71sThGE19

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #377994)

With the chromium codebase we found getUniqueVRegDef returns null as it could normally.
I'd add a check for that.

137–138 ↗

(On Diff #377994)

Ditto.

[craig.topper]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #377994)

Since this is running on SSA form, that should be getVRegDef. Is returning null because it is a physical register or for some other reason?

[jaykang10]

llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp

129 ↗

(On Diff #377994)

Sorry for bug again...

The MI is deleted after visiting all instructions so there are multiple definitions with same register after transformation and it breaks SSA form... I found the problem when I implemented https://reviews.llvm.org/D110841... I missed to update this patch with it... I am sorry for that.

I need to check the bootstrap build. Once it is ok, I will let you know.

[jaykang10]

Fixed a bug.

• Create new virtual register for the definition of new AND instruction and replace old register by the new one to keep SSA form.

If you feel it is not good solution, please let me know. I am checking bootstrap build on AArch64 machine with this change.

[jaykang10]

The check-all with bootstrap build is ok on AArch64 machine. The reduced case is also passed.

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

[danielkiss]

@jaykang10

In D109963#3068209, @jaykang10 wrote:

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

Done, Chrome built with the latest patch. Thanks!

[jaykang10]

In D109963#3069293, @danielkiss wrote:

@jaykang10

In D109963#3068209, @jaykang10 wrote:

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

Done, Chrome built with the latest patch. Thanks!

Thanks for checking it! Let me push this patch.

[benshi001]

In D109963#3069351, @jaykang10 wrote:

In D109963#3069293, @danielkiss wrote:

@jaykang10

In D109963#3068209, @jaykang10 wrote:

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

Done, Chrome built with the latest patch. Thanks!

Thanks for checking it! Let me push this patch.

Are there any more blames from build machines? Can I do my rebase work in https://reviews.llvm.org/D111034 ?

[dmgreen]

In D109963#3079967, @benshi001 wrote:

In D109963#3069351, @jaykang10 wrote:

In D109963#3069293, @danielkiss wrote:

@jaykang10

In D109963#3068209, @jaykang10 wrote:

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

Done, Chrome built with the latest patch. Thanks!

Thanks for checking it! Let me push this patch.

Are there any more blames from build machines? Can I do my rebase work in https://reviews.llvm.org/D111034 ?

I've not heard any issues, it sounds good to go ahead. Were the problems the same as were fixed here?

[benshi001]

In D109963#3080014, @dmgreen wrote:

In D109963#3079967, @benshi001 wrote:

In D109963#3069351, @jaykang10 wrote:

In D109963#3069293, @danielkiss wrote:

@jaykang10

In D109963#3068209, @jaykang10 wrote:

@danielkiss If possible, can you check the compiler crash on your side with latest update of this patch please?

Done, Chrome built with the latest patch. Thanks!

Thanks for checking it! Let me push this patch.

Are there any more blames from build machines? Can I do my rebase work in https://reviews.llvm.org/D111034 ?

I've not heard any issues, it sounds good to go ahead. Were the problems the same as were fixed here?

Not sure if the problems the same as were fixed here, seems to be different, and I can not reproduce it locally. Maybe I should directly do "git push" and wait for blames?

llvm/lib/Target/AArch64/AArch64InstrInfo.td

 // FIXME: these aliases *are* canonical sometimes (when movz can't be
 // used). Actually, it seems to be working right now, but putting logical_immXX
 // here is a bit dodgy on the AsmParser side too.
 def : InstAlias<"mov $dst, $imm", (ORRWri GPR32sp:$dst, WZR,
                                           logical_imm32:$imm), 0>;
 def : InstAlias<"mov $dst, $imm", (ORRXri GPR64sp:$dst, XZR,
                                           logical_imm64:$imm), 0>;
 
+// If the constant operand of AND(Immediate) is not valid bitmask immediate,
+// try to split it into two valid bitmask immediate.
+def and_split_bitmask_imm64 : ImmLeaf<i64, [{
+  uint64_t UImm64 = static_cast<uint64_t>(Imm);
+  if (AArch64_AM::isLogicalImmediate(UImm64, 64))
+    return false;
+
+  auto isSingleInstImm64 = [](uint64_t Imm) {
+    uint64_t Mask = APInt(0xFFFF, 16).getZExtValue();
+    for (unsigned i = 0; i < 4; i++) {
+      Mask <<= i * 16;
+      // This immediate can be suitable for single MOV instruction.
+      if ((Imm & Mask) == Imm)
+        return true;
+    }
+    return false;
+  };
+
+  if (isSingleInstImm64(UImm64))
+    return false;
+  if (isSingleInstImm64(~UImm64))
+    return false;
+
+  // The bitmask immediate consists of consecutive ones.  Let's say there is
+  // constant 0b00000000001000000000010000000000 which does not consist of
+  // consecutive ones. We can split it in to two bitmask immediate like
+  // 0b00000000001111111111110000000000 and 0b11111111111000000000011111111111.
+  // If we do AND with these two bitmask immediate, we can see original one.
+  unsigned LowestBitSet = countTrailingZeros(UImm64);
+  unsigned HighestBitSet = Log2_64(UImm64);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint64_t NewImm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  // Creat a mask which is filled with one outside the position of lowest bit
+  // set and the position of highest bit set.
+  uint64_t NewImm2 = UImm64 | ~NewImm1;
+
+  // If the splitted value is not valid bitmask immediate, do not split this
+  // constant.
+  if (!AArch64_AM::isLogicalImmediate(NewImm2, 64))
+    return false;
+  return true;
+}]>;
+
+def and_split_bitmask_imm32 : ImmLeaf<i32, [{
+  uint32_t UImm32 = static_cast<uint32_t>(Imm);
+  if (AArch64_AM::isLogicalImmediate(UImm32, 32))
+    return false;
+
+  auto isSingleInstImm32 = [](uint32_t Imm) {
+    uint32_t Mask = APInt(0xFFFF, 16).getZExtValue();
+    for (unsigned i = 0; i < 2; i++) {
+      Mask <<= i * 16;
+      // This immediate can be suitable for single MOV instruction.
+      if ((Imm & Mask) == Imm)
+        return true;
+    }
+    return false;
+  };
+
+  if (isSingleInstImm32(UImm32))
+    return false;
+  if (isSingleInstImm32(~UImm32))
+    return false;
+
+  // The bitmask immediate consists of consecutive ones.  Let's say there is
+  // constant 0b00000000001000000000010000000000 which does not consist of
+  // consecutive ones. We can split it in to two bitmask immediate like
+  // 0b00000000001111111111110000000000 and 0b11111111111000000000011111111111.
+  // If we do AND with these two bitmask immediate, we can see original one.
+  unsigned LowestBitSet = countTrailingZeros(UImm32);
+  unsigned HighestBitSet = Log2_64(UImm32);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint32_t NewImm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  // Creat a mask which is filled with one outside the position of lowest bit
+  // set and the position of highest bit set.
+  uint32_t NewImm2 = UImm32 | ~NewImm1;
+
+  // If the splitted value is not valid bitmask immediate, do not split this
+  // constant.
+  if (!AArch64_AM::isLogicalImmediate(NewImm2, 32))
+    return false;
+  return true;
+}]>;
+
+def first_bitmask_imm64_XFORM : SDNodeXForm<imm, [{
+  uint64_t Imm = N->getZExtValue();
+  unsigned LowestBitSet = countTrailingZeros(Imm);
+  unsigned HighestBitSet = Log2_64(Imm);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint64_t Imm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  uint64_t Imm1Enc = AArch64_AM::encodeLogicalImmediate(Imm1, 64);
+  return CurDAG->getTargetConstant(Imm1Enc, SDLoc(N), MVT::i64);
+}]>;
+
+def second_bitmask_imm64_XFORM : SDNodeXForm<imm, [{
+  uint64_t Imm = N->getZExtValue();
+  unsigned LowestBitSet = countTrailingZeros(Imm);
+  unsigned HighestBitSet = Log2_64(Imm);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint64_t Imm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  // Creat a mask which is filled with one outside the position of lowest bit
+  // set and the position of highest bit set.
+  uint64_t Imm2 = Imm | ~Imm1;
+  uint64_t Imm2Enc = AArch64_AM::encodeLogicalImmediate(Imm2, 64);
+  return CurDAG->getTargetConstant(Imm2Enc, SDLoc(N), MVT::i64);
+}]>;
+
+def first_bitmask_imm32_XFORM : SDNodeXForm<imm, [{
+  uint32_t Imm = N->getZExtValue();
+  unsigned LowestBitSet = countTrailingZeros(Imm);
+  unsigned HighestBitSet = Log2_64(Imm);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint32_t Imm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  uint32_t Imm1Enc = AArch64_AM::encodeLogicalImmediate(Imm1, 32);
+  return CurDAG->getTargetConstant(Imm1Enc, SDLoc(N), MVT::i32);
+}]>;
+
+def second_bitmask_imm32_XFORM : SDNodeXForm<imm, [{
+  uint32_t Imm = N->getZExtValue();
+  unsigned LowestBitSet = countTrailingZeros(Imm);
+  unsigned HighestBitSet = Log2_64(Imm);
+
+  // Creat a mask which is filled with one from the position of lowest bit set
+  // to the position of highest bit set.
+  uint32_t Imm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  // Creat a mask which is filled with one outside the position of lowest bit
+  // set and the position of highest bit set.
+  uint32_t Imm2 = Imm | ~Imm1;
+  uint32_t Imm2Enc = AArch64_AM::encodeLogicalImmediate(Imm2, 32);
+  return CurDAG->getTargetConstant(Imm2Enc, SDLoc(N), MVT::i32);
+}]>;
+
+def first_bitmask_imm64 : Operand<i64>, IntImmLeaf<i64, [{
+  return AArch64_AM::isLogicalImmediate(Imm.getZExtValue(), 64);
+}], first_bitmask_imm64_XFORM> {}
+
+def second_bitmask_imm64 : Operand<i64>, IntImmLeaf<i64, [{
+  return AArch64_AM::isLogicalImmediate(Imm.getZExtValue(), 64);
+}], second_bitmask_imm64_XFORM> {}
+
+def first_bitmask_imm32 : Operand<i32>, IntImmLeaf<i32, [{
+  return AArch64_AM::isLogicalImmediate(Imm.getZExtValue(), 32);
+}], first_bitmask_imm32_XFORM> {}
+
+def second_bitmask_imm32 : Operand<i32>, IntImmLeaf<i32, [{
+  return AArch64_AM::isLogicalImmediate(Imm.getZExtValue(), 32);
+}], second_bitmask_imm32_XFORM> {}
+
+def : Pat<(i64 (and GPR64:$src, (i64 and_split_bitmask_imm64:$imm))),
+          (ANDXri (ANDXri GPR64:$src, (i64 (first_bitmask_imm64:$imm))),
+                  (i64 (second_bitmask_imm64:$imm)))>;
+
+def : Pat<(i32 (and GPR32:$src, (i32 and_split_bitmask_imm32:$imm))),
+          (ANDWri (ANDWri GPR32:$src, (i32 (first_bitmask_imm32:$imm))),
+                  (i32 (second_bitmask_imm32:$imm)))>;
 
 // (register)
 defm ANDS : LogicalRegS<0b11, 0, "ands", AArch64and_flag>;
 defm BICS : LogicalRegS<0b11, 1, "bics",
                         BinOpFrag<(AArch64and_flag node:$LHS, (not node:$RHS))>>;
 defm AND  : LogicalReg<0b00, 0, "and", and>;
 defm BIC  : LogicalReg<0b00, 1, "bic",
                        BinOpFrag<(and node:$LHS, (not node:$RHS))>>;

llvm/test/CodeGen/AArch64/aarch64-split-and-bitmask-immediate.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s
+
+define i8 @test(i32 %a) {
+; CHECK-LABEL: test:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    and w8, w0, #0x3ffc00
+; CHECK-NEXT:    and w8, w8, #0xffe007ff
+; CHECK-NEXT:    cmp w8, #1024
+; CHECK-NEXT:    cset w0, eq
+; CHECK-NEXT:    ret
+entry:
+  %and = and i32 %a, 2098176
+  %cmp = icmp eq i32 %and, 1024
+  %conv = zext i1 %cmp to i8
+  ret i8 %conv
+}

llvm/test/CodeGen/AArch64/arm64-ccmp.ll

 ; Test case distilled from 126.gcc.
 ; The phi in sw.bb.i.i gets multiple operands for the %entry predecessor.
 define void @build_modify_expr() nounwind ssp {
 ; CHECK-LABEL: build_modify_expr:
 ; CHECK:       ; %bb.0: ; %entry
 ; CHECK-NEXT:    cmp w8, #37
 ; CHECK-NEXT:    mov w8, #1
 ; CHECK-NEXT:    lsl x8, x8, xzr
-; CHECK-NEXT:    mov x9, #31
-; CHECK-NEXT:    movk x9, #48, lsl #32
-; CHECK-NEXT:    and x8, x8, x9
+; CHECK-NEXT:    and x8, x8, #0x3f
+; CHECK-NEXT:    and x8, x8, #0xffffffffffffffdf
 ; CHECK-NEXT:    ccmp x8, #0, #4, ls
 ; CHECK-NEXT:    b.eq LBB11_2
 ; CHECK-NEXT:  ; %bb.1: ; %if.end85
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  LBB11_2: ; %sw.bb.i.i.preheader
 ; CHECK-NEXT:    ; implicit-def: $x8
 ; CHECK-NEXT:  LBB11_3: ; %sw.bb.i.i
 ; CHECK-NEXT:    ; =>This Inner Loop Header: Depth=1

llvm/test/CodeGen/AArch64/bitfield-insert.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s
 
 ; First, a simple example from Clang. The registers could plausibly be
 ; different, but probably won't be.
 
 %struct.foo = type { i8, [2 x i8], i8 }
 
 define [1 x i64] @from_clang([1 x i64] %f.coerce, i32 %n) nounwind readnone {
 ; CHECK-LABEL: from_clang:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    mov w8, #135
-; CHECK-NEXT:    and w8, w0, w8
+; CHECK-NEXT:    and w8, w0, #0xff
+; CHECK-NEXT:    and w8, w8, #0xffffff87
 ; CHECK-NEXT:    bfi w8, w1, #3, #4
 ; CHECK-NEXT:    and x9, x0, #0xffffff00
 ; CHECK-NEXT:    orr x0, x8, x9
 ; CHECK-NEXT:    ret
 entry:
   %f.coerce.fca.0.extract = extractvalue [1 x i64] %f.coerce, 0
   %tmp.sroa.0.0.extract.trunc = trunc i64 %f.coerce.fca.0.extract to i32
   %bf.value = shl i32 %n, 3
; CHECK-NEXT:    ret
   ret void
 }
 
 define void @test_32bit_masked(i32 *%existing, i32 *%new) {
 ; CHECK-LABEL: test_32bit_masked:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ldr w8, [x0]
 ; CHECK-NEXT:    ldr w9, [x1]
-; CHECK-NEXT:    mov w10, #135
-; CHECK-NEXT:    and w8, w8, w10
+; CHECK-NEXT:    and w8, w8, #0xff
+; CHECK-NEXT:    and w8, w8, #0xffffff87
 ; CHECK-NEXT:    bfi w8, w9, #3, #4
 ; CHECK-NEXT:    str w8, [x0]
 ; CHECK-NEXT:    ret
   %oldval = load volatile i32, i32* %existing
   %oldval_keep = and i32 %oldval, 135 ; = 0x87
 
   %newval = load volatile i32, i32* %new
   %newval_shifted = shl i32 %newval, 3

 }
 
 ; Neither mask is a contiguous set of 1s. BFI can't be used
 define void @test_32bit_badmask(i32 *%existing, i32 *%new) {
 ; CHECK-LABEL: test_32bit_badmask:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ldr w8, [x0]
 ; CHECK-NEXT:    ldr w9, [x1]
-; CHECK-NEXT:    mov w10, #135
-; CHECK-NEXT:    mov w11, #632
-; CHECK-NEXT:    and w8, w8, w10
-; CHECK-NEXT:    and w9, w11, w9, lsl #3
+; CHECK-NEXT:    mov w10, #632
+; CHECK-NEXT:    and w8, w8, #0xff
+; CHECK-NEXT:    and w8, w8, #0xffffff87
+; CHECK-NEXT:    and w9, w10, w9, lsl #3
 ; CHECK-NEXT:    orr w8, w8, w9
 ; CHECK-NEXT:    str w8, [x0]
 ; CHECK-NEXT:    ret
   %oldval = load volatile i32, i32* %existing
   %oldval_keep = and i32 %oldval, 135 ; = 0x87
 
   %newval = load volatile i32, i32* %new
   %newval_shifted = shl i32 %newval, 3
   %newval_masked = and i32 %newval_shifted, 632 ; = 0x278
 
   %combined = or i32 %oldval_keep, %newval_masked
   store volatile i32 %combined, i32* %existing
 
   ret void
 }
 
 ; Ditto
 define void @test_64bit_badmask(i64 *%existing, i64 *%new) {
 ; CHECK-LABEL: test_64bit_badmask:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ldr x8, [x0]
 ; CHECK-NEXT:    ldr x9, [x1]
-; CHECK-NEXT:    mov w10, #135
-; CHECK-NEXT:    and x8, x8, x10
-; CHECK-NEXT:    lsl w9, w9, #3
 ; CHECK-NEXT:    mov w10, #664
+; CHECK-NEXT:    and x8, x8, #0xff
+; CHECK-NEXT:    lsl w9, w9, #3
+; CHECK-NEXT:    and x8, x8, #0xffffffffffffff87
 ; CHECK-NEXT:    and x9, x9, x10
 ; CHECK-NEXT:    orr x8, x8, x9
 ; CHECK-NEXT:    str x8, [x0]
 ; CHECK-NEXT:    ret
   %oldval = load volatile i64, i64* %existing
   %oldval_keep = and i64 %oldval, 135 ; = 0x87
 
   %newval = load volatile i64, i64* %new

llvm/test/CodeGen/AArch64/unfold-masked-merge-scalar-constmask-innerouter.ll

; CHECK-NEXT:    ret
   ret i32 %r
 }
 
 ; Various bad variants
 
 define i32 @n0_badconstmask(i32 %x, i32 %y) {
 ; CHECK-LABEL: n0_badconstmask:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov w9, #256
-; CHECK-NEXT:    movk w9, #65280, lsl #16
+; CHECK-NEXT:    and w9, w1, #0xffffff00
 ; CHECK-NEXT:    and w8, w0, #0xffff00
-; CHECK-NEXT:    and w9, w1, w9
+; CHECK-NEXT:    and w9, w9, #0xff0001ff
 ; CHECK-NEXT:    orr w0, w8, w9
 ; CHECK-NEXT:    ret
   %mx = and i32 %x, 16776960
   %my = and i32 %y, -16776960 ; instead of -16776961
   %r = or i32 %mx, %my
   ret i32 %r
 }
 

llvm/test/CodeGen/AArch64/unfold-masked-merge-scalar-constmask-interleavedbytehalves.ll

; CHECK-NEXT:    ret
   %my = and i8 %y, -16
   %r = or i8 %mx, %my
   ret i8 %r
 }
 
 define i16 @out16_constmask(i16 %x, i16 %y) {
 ; CHECK-LABEL: out16_constmask:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov w8, #3855
-; CHECK-NEXT:    mov w9, #-3856
-; CHECK-NEXT:    and w8, w0, w8
-; CHECK-NEXT:    and w9, w1, w9
+; CHECK-NEXT:    and w8, w0, #0xfff
+; CHECK-NEXT:    and w9, w1, #0xfffffff0
+; CHECK-NEXT:    and w8, w8, #0xffffff0f
+; CHECK-NEXT:    and w9, w9, #0xfffff0ff
 ; CHECK-NEXT:    orr w0, w8, w9
 ; CHECK-NEXT:    ret
   %mx = and i16 %x, 3855
   %my = and i16 %y, -3856
   %r = or i16 %mx, %my
   ret i16 %r
 }
 
; CHECK-NEXT:    ret
   %r = xor i8 %n1, %y
   ret i8 %r
 }
 
 define i16 @in16_constmask(i16 %x, i16 %y) {
 ; CHECK-LABEL: in16_constmask:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    eor w8, w0, w1
-; CHECK-NEXT:    mov w9, #3855
-; CHECK-NEXT:    and w8, w8, w9
+; CHECK-NEXT:    and w8, w8, #0xfff
+; CHECK-NEXT:    and w8, w8, #0xffffff0f
 ; CHECK-NEXT:    eor w0, w8, w1
 ; CHECK-NEXT:    ret
   %n0 = xor i16 %x, %y
   %n1 = and i16 %n0, 3855
   %r = xor i16 %n1, %y
   ret i16 %r
 }