[AArch64] Allow logical immediates to have all-1 in top bits

[MaskRay]

Summary

So that constant expressions like the following are permitted:

and w0, w0, #~(0xfe<<24)
and w1, w1, #~(0xff<<24)

The behavior matches GNU as (opcodes/aarch64-opc.c:aarch64_logical_immediate_p).

[MaskRay]

Add a comment.

[samparker]

It seems a bit odd to add assembly support for something which we also can't disassemble? It seems like a basic self-test that we should have.

[MaskRay]

In D75885#1913963, @samparker wrote:

It seems a bit odd to add assembly support for something which we also can't disassemble? It seems like a basic self-test that we should have.

Not odd. We can't assemble

and w0, w0, #~(0xfe<<24)
and w1, w1, #~(0xff<<24)

This patch teaches MC to assemble them. It is just that the disassembly is not a round trip.

For dupm z0.h, #0x7f00, it is indeed odd. GNU as behaves the same, though. I know too little about SVE to fix it properly by myself. I hope more experienced contributors can do that as a follow-up.

[MaskRay]

Friendly ping

[samparker]

I think you should try to figure what the issue is with SVE... It's not good practise to scratch your particular itch and then not fix the bug it exposes too.

[MaskRay]

In D75885#1926549, @samparker wrote:

I think you should try to figure what the issue is with SVE... It's not good practise to scratch your particular itch and then not fix the bug it exposes too.

May I say that this suggests a quality of implementation issue of the SVE patches themselves? GNU as has the same problem... The organization of code makes is sorta clear that if we intend to fix SVE, it will be a separate patch.

[MaskRay]

Ping:)

As I said, I think the SVE problem is very likely its own QoI problem (GNU as has the same issue). I hope an experienced contributor can fix that in a subsequent change, but it should not block this change.

[sdesmalen]

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

761

Is this the same as:

uint64_t Upper = UINT64_C(-1) << (sizeof(T) * 8);

?

llvm/test/MC/AArch64/SVE/mov.s

210

Disassembling to dupm z0.h, #0x7f00 makes sense here.

The mov alias is preferred when SVEMoveMaskPreferred is true. This instructino would splat 0x7f00 into each 16-bit lane of z0, resulting in 0x7f007f007f007f00. For this input, SVEMoveMaskPreferred returns false, so dupm is the correct disassembly.

So I'd say the fixme can be removed.

[MaskRay]

Delete FIXME. According to sdesmalen, "Disassembling to dupm z0.h, #0x7f00 makes sense here."

[MaskRay]

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

761

http://eel.is/c++draft//expr.shift

The behavior is undefined if the right operand is negative, or greater than or equal to the width of the promoted left operand.

This avoids direct left shift by 64.

[sdesmalen]

LGTM

[MaskRay]

Thanks for the review!

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

public:
   bool isLogicalImm() const {
     if (!isImm())
       return false;
     const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(getImm());
     if (!MCE)
       return false;
 
     int64_t Val = MCE->getValue();
-    int64_t SVal = std::make_signed_t<T>(Val);
-    int64_t UVal = std::make_unsigned_t<T>(Val);
-    if (Val != SVal && Val != UVal)
+    // Avoid left shift by 64 directly.
+    uint64_t Upper = UINT64_C(-1) << (sizeof(T) * 4) << (sizeof(T) * 4);
+    // Allow all-0 or all-1 in top bits to permit bitwise NOT.
+    if ((Val & Upper) && (Val & Upper) != Upper)
       return false;
 
-    return AArch64_AM::isLogicalImmediate(UVal, sizeof(T) * 8);
+    return AArch64_AM::isLogicalImmediate(Val & ~Upper, sizeof(T) * 8);
   }
 
   bool isShiftedImm() const { return Kind == k_ShiftedImm; }
 
   /// Returns the immediate value as a pair of (imm, shift) if the immediate is
   /// a shifted immediate by value 'Shift' or '0', or if it is an unshifted
   /// immediate that can be shifted by 'Shift'.
   template <unsigned Width>

llvm/test/MC/AArch64/SVE/mov-diagnostics.s

 // CHECK-NEXT: mov z0.b, #256
 // CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 mov z0.b, #1, lsl #8
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: immediate must be an integer in range [-128, 255]
 // CHECK-NEXT: mov z0.b, #1, lsl #8
 // CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
-mov z0.h, #-33024
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: immediate must be an integer in range [-128, 127] or a multiple of 256 in range [-32768, 65280]
-// CHECK-NEXT: mov z0.h, #-33024
-// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
-
-mov z0.h, #-32769
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: immediate must be an integer in range [-128, 127] or a multiple of 256 in range [-32768, 65280]
-// CHECK-NEXT: mov z0.h, #-32769
-// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
-
 mov z0.h, #-129, lsl #8
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: immediate must be an integer in range [-128, 127] or a multiple of 256 in range [-32768, 65280]
 // CHECK-NEXT: mov z0.h, #-129, lsl #8
 // CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Note: 65281 is a valid logical immediate.
 mov z0.h, #65282
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: immediate must be an integer in range [-128, 127] or a multiple of 256 in range [-32768, 65280]

llvm/test/MC/AArch64/SVE/mov.s

 // CHECK-UNKNOWN: 00 f0 78 25 <unknown>
 
 mov     z0.h, #65280
 // CHECK-INST: mov    z0.h, #-256
 // CHECK-ENCODING: [0xe0,0xff,0x78,0x25]
 // CHECK-ERROR: instruction requires: sve
 // CHECK-UNKNOWN: e0 ff 78 25 <unknown>
 
+mov     z0.h, #-33024
+// CHECK-INST: dupm z0.h, #0x7f00
+// CHECK-ENCODING: [0xc0,0x44,0xc0,0x05]
+// CHECK-ERROR: instruction requires: sve
+// CHECK-UNKNOWN: c0 44 c0 05 <unknown>
+
+mov     z0.h, #-32769
+// CHECK-INST: mov z0.h, #32767
+// CHECK-ENCODING: [0xc0,0x05,0xc0,0x05]
+// CHECK-ERROR: instruction requires: sve
+// CHECK-UNKNOWN: c0 05 c0 05 <unknown>
+
 mov     z0.s, #-32769
 // CHECK-INST: mov     z0.s, #0xffff7fff
 // CHECK-ENCODING: [0xc0,0x83,0xc0,0x05]
 // CHECK-ERROR: instruction requires: sve
 // CHECK-UNKNOWN: c0 83 c0 05 <unknown>
 
 mov     z0.s, #32768
 // CHECK-INST: mov     z0.s, #32768

llvm/test/MC/AArch64/arm64-logical-encoding.s

 ; CHECK: orn w1, w2, w3, lsl #7      ; encoding: [0x41,0x1c,0x23,0x2a]
 ; CHECK: orn x1, x2, x3, lsl #7      ; encoding: [0x41,0x1c,0x23,0xaa]
 ; CHECK: orn w1, w2, w3, lsr #7      ; encoding: [0x41,0x1c,0x63,0x2a]
 ; CHECK: orn x1, x2, x3, lsr #7      ; encoding: [0x41,0x1c,0x63,0xaa]
 ; CHECK: orn w1, w2, w3, asr #7      ; encoding: [0x41,0x1c,0xa3,0x2a]
 ; CHECK: orn x1, x2, x3, asr #7      ; encoding: [0x41,0x1c,0xa3,0xaa]
 ; CHECK: orn w1, w2, w3, ror #7      ; encoding: [0x41,0x1c,0xe3,0x2a]
 ; CHECK: orn x1, x2, x3, ror #7      ; encoding: [0x41,0x1c,0xe3,0xaa]
+
+;; Allow all-1 in top bits.
+  and w0, w0, #~(0xfe<<24)
+  and w1, w1, #~(0xff<<24)
+
+; CHECK: and w0, w0, #0x1ffffff
+; CHECK: and w1, w1, #0xffffff