Skip to content

Commit 3f757a3

Browse files
nikicnikic
authored
[CodeGen] Remove ExpandInlineAsm hook (#156617)
This hook replaces inline asm with LLVM intrinsics. It was intended to match inline assembly implementations of bswap in libc headers and replace them more optimizable implementations. At this point, it has outlived its usefulness (see #156571 (comment)), as libc implementations no longer use inline assembly for this purpose. Additionally, it breaks the "black box" property of inline assembly, which some languages like Rust would like to guarantee. Fixes #156571.
1 parent 4d927a5 commit 3f757a3

File tree

10 files changed

+49
-180
lines changed

10 files changed

+49
-180
lines changed

‎llvm/include/llvm/CodeGen/TargetLowering.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5125,14 +5125,6 @@ class LLVM_ABI TargetLowering : public TargetLoweringBase {
51255125
// Inline Asm Support hooks
51265126
//
51275127

5128-
/// This hook allows the target to expand an inline asm call to be explicit
5129-
/// llvm code if it wants to. This is useful for turning simple inline asms
5130-
/// into LLVM intrinsics, which gives the compiler more information about the
5131-
/// behavior of the code.
5132-
virtual bool ExpandInlineAsm(CallInst *) const {
5133-
return false;
5134-
}
5135-
51365128
enum ConstraintType {
51375129
C_Register, // Constraint represents specific register(s).
51385130
C_RegisterClass, // Constraint represents any of register(s) in class.

‎llvm/lib/CodeGen/CodeGenPrepare.cpp‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2618,22 +2618,9 @@ static bool despeculateCountZeros(IntrinsicInst *CountZeros, LoopInfo &LI,
26182618
bool CodeGenPrepare::optimizeCallInst(CallInst *CI, ModifyDT &ModifiedDT) {
26192619
BasicBlock *BB = CI->getParent();
26202620

2621-
// Lower inline assembly if we can.
2622-
// If we found an inline asm expession, and if the target knows how to
2623-
// lower it to normal LLVM code, do so now.
2624-
if (CI->isInlineAsm()) {
2625-
if (TLI->ExpandInlineAsm(CI)) {
2626-
// Avoid invalidating the iterator.
2627-
CurInstIterator = BB->begin();
2628-
// Avoid processing instructions out of order, which could cause
2629-
// reuse before a value is defined.
2630-
SunkAddrs.clear();
2631-
return true;
2632-
}
2633-
// Sink address computing for memory operands into the block.
2634-
if (optimizeInlineAsmInst(CI))
2635-
return true;
2636-
}
2621+
// Sink address computing for memory operands into the block.
2622+
if (CI->isInlineAsm() && optimizeInlineAsmInst(CI))
2623+
return true;
26372624

26382625
// Align the pointer arguments to this call if the target thinks it's a good
26392626
// idea

‎llvm/lib/Target/ARM/ARMISelLowering.cpp‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20192,37 +20192,6 @@ bool ARMTargetLowering::SimplifyDemandedBitsForTargetNode(
2019220192
// ARM Inline Assembly Support
2019320193
//===----------------------------------------------------------------------===//
2019420194

20195-
bool ARMTargetLowering::ExpandInlineAsm(CallInst *CI) const {
20196-
// Looking for "rev" which is V6+.
20197-
if (!Subtarget->hasV6Ops())
20198-
return false;
20199-
20200-
InlineAsm *IA = cast<InlineAsm>(CI->getCalledOperand());
20201-
StringRef AsmStr = IA->getAsmString();
20202-
SmallVector<StringRef, 4> AsmPieces;
20203-
SplitString(AsmStr, AsmPieces, ";\n");
20204-
20205-
switch (AsmPieces.size()) {
20206-
default: return false;
20207-
case 1:
20208-
AsmStr = AsmPieces[0];
20209-
AsmPieces.clear();
20210-
SplitString(AsmStr, AsmPieces, " \t,");
20211-
20212-
// rev $0, $1
20213-
if (AsmPieces.size() == 3 && AsmPieces[0] == "rev" &&
20214-
AsmPieces[1] == "$0" && AsmPieces[2] == "$1" &&
20215-
IA->getConstraintString().starts_with("=l,l")) {
20216-
IntegerType *Ty = dyn_cast<IntegerType>(CI->getType());
20217-
if (Ty && Ty->getBitWidth() == 32)
20218-
return IntrinsicLowering::LowerToByteSwap(CI);
20219-
}
20220-
break;
20221-
}
20222-
20223-
return false;
20224-
}
20225-
2022620195
const char *ARMTargetLowering::LowerXConstraint(EVT ConstraintVT) const {
2022720196
// At this point, we have to lower this constraint to something else, so we
2022820197
// lower it to an "r" or "w". However, by doing this we will force the result

‎llvm/lib/Target/ARM/ARMISelLowering.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,8 +534,6 @@ class VectorType;
534534
const APInt &DemandedElts,
535535
TargetLoweringOpt &TLO) const override;
536536

537-
bool ExpandInlineAsm(CallInst *CI) const override;
538-
539537
ConstraintType getConstraintType(StringRef Constraint) const override;
540538

541539
/// Examine constraint string and operand type and determine a weight value.

‎llvm/lib/Target/X86/X86ISelLowering.cpp‎

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -61009,117 +61009,6 @@ bool X86TargetLowering::IsDesirableToPromoteOp(SDValue Op, EVT &PVT) const {
6100961009
// X86 Inline Assembly Support
6101061010
//===----------------------------------------------------------------------===//
6101161011

61012-
// Helper to match a string separated by whitespace.
61013-
static bool matchAsm(StringRef S, ArrayRef<const char *> Pieces) {
61014-
S = S.substr(S.find_first_not_of(" \t")); // Skip leading whitespace.
61015-
61016-
for (StringRef Piece : Pieces) {
61017-
if (!S.starts_with(Piece)) // Check if the piece matches.
61018-
return false;
61019-
61020-
S = S.substr(Piece.size());
61021-
StringRef::size_type Pos = S.find_first_not_of(" \t");
61022-
if (Pos == 0) // We matched a prefix.
61023-
return false;
61024-
61025-
S = S.substr(Pos);
61026-
}
61027-
61028-
return S.empty();
61029-
}
61030-
61031-
static bool clobbersFlagRegisters(const SmallVector<StringRef, 4> &AsmPieces) {
61032-
61033-
if (AsmPieces.size() == 3 || AsmPieces.size() == 4) {
61034-
if (llvm::is_contained(AsmPieces, "~{cc}") &&
61035-
llvm::is_contained(AsmPieces, "~{flags}") &&
61036-
llvm::is_contained(AsmPieces, "~{fpsr}")) {
61037-
61038-
if (AsmPieces.size() == 3)
61039-
return true;
61040-
else if (llvm::is_contained(AsmPieces, "~{dirflag}"))
61041-
return true;
61042-
}
61043-
}
61044-
return false;
61045-
}
61046-
61047-
bool X86TargetLowering::ExpandInlineAsm(CallInst *CI) const {
61048-
InlineAsm *IA = cast<InlineAsm>(CI->getCalledOperand());
61049-
61050-
StringRef AsmStr = IA->getAsmString();
61051-
61052-
IntegerType *Ty = dyn_cast<IntegerType>(CI->getType());
61053-
if (!Ty || Ty->getBitWidth() % 16 != 0)
61054-
return false;
61055-
61056-
// TODO: should remove alternatives from the asmstring: "foo {a|b}" -> "foo a"
61057-
SmallVector<StringRef, 4> AsmPieces;
61058-
SplitString(AsmStr, AsmPieces, ";\n");
61059-
61060-
switch (AsmPieces.size()) {
61061-
default: return false;
61062-
case 1:
61063-
// FIXME: this should verify that we are targeting a 486 or better. If not,
61064-
// we will turn this bswap into something that will be lowered to logical
61065-
// ops instead of emitting the bswap asm. For now, we don't support 486 or
61066-
// lower so don't worry about this.
61067-
// bswap $0
61068-
if (matchAsm(AsmPieces[0], {"bswap", "$0"}) ||
61069-
matchAsm(AsmPieces[0], {"bswapl", "$0"}) ||
61070-
matchAsm(AsmPieces[0], {"bswapq", "$0"}) ||
61071-
matchAsm(AsmPieces[0], {"bswap", "${0:q}"}) ||
61072-
matchAsm(AsmPieces[0], {"bswapl", "${0:q}"}) ||
61073-
matchAsm(AsmPieces[0], {"bswapq", "${0:q}"})) {
61074-
// No need to check constraints, nothing other than the equivalent of
61075-
// "=r,0" would be valid here.
61076-
return IntrinsicLowering::LowerToByteSwap(CI);
61077-
}
61078-
61079-
// rorw $$8, ${0:w} --> llvm.bswap.i16
61080-
if (CI->getType()->isIntegerTy(16) &&
61081-
IA->getConstraintString().starts_with("=r,0,") &&
61082-
(matchAsm(AsmPieces[0], {"rorw", "$$8,", "${0:w}"}) ||
61083-
matchAsm(AsmPieces[0], {"rolw", "$$8,", "${0:w}"}))) {
61084-
AsmPieces.clear();
61085-
StringRef ConstraintsStr = IA->getConstraintString();
61086-
SplitString(StringRef(ConstraintsStr).substr(5), AsmPieces, ",");
61087-
array_pod_sort(AsmPieces.begin(), AsmPieces.end());
61088-
if (clobbersFlagRegisters(AsmPieces))
61089-
return IntrinsicLowering::LowerToByteSwap(CI);
61090-
}
61091-
break;
61092-
case 3:
61093-
if (CI->getType()->isIntegerTy(32) &&
61094-
IA->getConstraintString().starts_with("=r,0,") &&
61095-
matchAsm(AsmPieces[0], {"rorw", "$$8,", "${0:w}"}) &&
61096-
matchAsm(AsmPieces[1], {"rorl", "$$16,", "$0"}) &&
61097-
matchAsm(AsmPieces[2], {"rorw", "$$8,", "${0:w}"})) {
61098-
AsmPieces.clear();
61099-
StringRef ConstraintsStr = IA->getConstraintString();
61100-
SplitString(StringRef(ConstraintsStr).substr(5), AsmPieces, ",");
61101-
array_pod_sort(AsmPieces.begin(), AsmPieces.end());
61102-
if (clobbersFlagRegisters(AsmPieces))
61103-
return IntrinsicLowering::LowerToByteSwap(CI);
61104-
}
61105-
61106-
if (CI->getType()->isIntegerTy(64)) {
61107-
InlineAsm::ConstraintInfoVector Constraints = IA->ParseConstraints();
61108-
if (Constraints.size() >= 2 &&
61109-
Constraints[0].Codes.size() == 1 && Constraints[0].Codes[0] == "A" &&
61110-
Constraints[1].Codes.size() == 1 && Constraints[1].Codes[0] == "0") {
61111-
// bswap %eax / bswap %edx / xchgl %eax, %edx -> llvm.bswap.i64
61112-
if (matchAsm(AsmPieces[0], {"bswap", "%eax"}) &&
61113-
matchAsm(AsmPieces[1], {"bswap", "%edx"}) &&
61114-
matchAsm(AsmPieces[2], {"xchgl", "%eax,", "%edx"}))
61115-
return IntrinsicLowering::LowerToByteSwap(CI);
61116-
}
61117-
}
61118-
break;
61119-
}
61120-
return false;
61121-
}
61122-
6112361012
static X86::CondCode parseConstraintCode(llvm::StringRef Constraint) {
6112461013
X86::CondCode Cond = StringSwitch<X86::CondCode>(Constraint)
6112561014
.Case("{@cca}", X86::COND_A)

‎llvm/lib/Target/X86/X86ISelLowering.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,8 +1364,6 @@ namespace llvm {
13641364

13651365
SDValue getReturnAddressFrameIndex(SelectionDAG &DAG) const;
13661366

1367-
bool ExpandInlineAsm(CallInst *CI) const override;
1368-
13691367
ConstraintType getConstraintType(StringRef Constraint) const override;
13701368

13711369
/// Examine constraint string and operand type and determine a weight value.

‎llvm/test/CodeGen/ARM/bswap-inline-asm.ll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
; RUN: llc < %s -mtriple=arm-apple-darwin -mattr=+v6 | FileCheck %s
22

3+
; rev inline assembly should be preserved as-is.
4+
35
define i32 @t1(i32 %x) nounwind {
46
; CHECK-LABEL: t1:
5-
; CHECK-NOT: InlineAsm
7+
; CHECK: InlineAsm
68
; CHECK: rev
79
%asmtmp = tail call i32 asm "rev $0, $1\0A", "=l,l"(i32 %x) nounwind
810
ret i32 %asmtmp

‎llvm/test/CodeGen/X86/bswap-inline-asm.ll‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
22
; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck %s
33

4+
; bswap inline assembly should be preserved as-is.
5+
46
define i64 @foo(i64 %x) nounwind {
57
; CHECK-LABEL: foo:
68
; CHECK: ## %bb.0:
79
; CHECK-NEXT: movq %rdi, %rax
10+
; CHECK-NEXT: ## InlineAsm Start
811
; CHECK-NEXT: bswapq %rax
12+
; CHECK-NEXT: ## InlineAsm End
913
; CHECK-NEXT: retq
1014
%asmtmp = tail call i64 asm "bswap $0", "=r,0,~{dirflag},~{fpsr},~{flags}"(i64 %x) nounwind
1115
ret i64 %asmtmp
@@ -15,7 +19,9 @@ define i64 @bar(i64 %x) nounwind {
1519
; CHECK-LABEL: bar:
1620
; CHECK: ## %bb.0:
1721
; CHECK-NEXT: movq %rdi, %rax
22+
; CHECK-NEXT: ## InlineAsm Start
1823
; CHECK-NEXT: bswapq %rax
24+
; CHECK-NEXT: ## InlineAsm End
1925
; CHECK-NEXT: retq
2026
%asmtmp = tail call i64 asm "bswapq ${0:q}", "=r,0,~{dirflag},~{fpsr},~{flags}"(i64 %x) nounwind
2127
ret i64 %asmtmp
@@ -25,16 +31,20 @@ define i32 @pen(i32 %x) nounwind {
2531
; CHECK-LABEL: pen:
2632
; CHECK: ## %bb.0:
2733
; CHECK-NEXT: movl %edi, %eax
34+
; CHECK-NEXT: ## InlineAsm Start
2835
; CHECK-NEXT: bswapl %eax
36+
; CHECK-NEXT: ## InlineAsm End
2937
; CHECK-NEXT: retq
30-
%asmtmp = tail call i32 asm "bswapl ${0:q}", "=r,0,~{dirflag},~{fpsr},~{flags}"(i32 %x) nounwind
38+
%asmtmp = tail call i32 asm "bswapl ${0:k}", "=r,0,~{dirflag},~{fpsr},~{flags}"(i32 %x) nounwind
3139
ret i32 %asmtmp
3240
}
3341

3442
define zeroext i16 @s16(i16 zeroext %x) nounwind {
3543
; CHECK-LABEL: s16:
3644
; CHECK: ## %bb.0:
37-
; CHECK-NEXT: rolw $8, %di
45+
; CHECK-NEXT: ## InlineAsm Start
46+
; CHECK-NEXT: rorw $8, %di
47+
; CHECK-NEXT: ## InlineAsm End
3848
; CHECK-NEXT: movzwl %di, %eax
3949
; CHECK-NEXT: retq
4050
%asmtmp = tail call i16 asm "rorw $$8, ${0:w}", "=r,0,~{dirflag},~{fpsr},~{flags},~{cc}"(i16 %x) nounwind
@@ -44,7 +54,9 @@ define zeroext i16 @s16(i16 zeroext %x) nounwind {
4454
define zeroext i16 @t16(i16 zeroext %x) nounwind {
4555
; CHECK-LABEL: t16:
4656
; CHECK: ## %bb.0:
47-
; CHECK-NEXT: rolw $8, %di
57+
; CHECK-NEXT: ## InlineAsm Start
58+
; CHECK-NEXT: rorw $8, %di
59+
; CHECK-NEXT: ## InlineAsm End
4860
; CHECK-NEXT: movzwl %di, %eax
4961
; CHECK-NEXT: retq
5062
%asmtmp = tail call i16 asm "rorw $$8, ${0:w}", "=r,0,~{cc},~{dirflag},~{fpsr},~{flags}"(i16 %x) nounwind
@@ -54,7 +66,9 @@ define zeroext i16 @t16(i16 zeroext %x) nounwind {
5466
define zeroext i16 @u16(i16 zeroext %x) nounwind {
5567
; CHECK-LABEL: u16:
5668
; CHECK: ## %bb.0:
69+
; CHECK-NEXT: ## InlineAsm Start
5770
; CHECK-NEXT: rolw $8, %di
71+
; CHECK-NEXT: ## InlineAsm End
5872
; CHECK-NEXT: movzwl %di, %eax
5973
; CHECK-NEXT: retq
6074
%asmtmp = tail call i16 asm "rolw $$8, ${0:w}", "=r,0,~{dirflag},~{fpsr},~{flags},~{cc}"(i16 %x) nounwind
@@ -64,7 +78,9 @@ define zeroext i16 @u16(i16 zeroext %x) nounwind {
6478
define zeroext i16 @v16(i16 zeroext %x) nounwind {
6579
; CHECK-LABEL: v16:
6680
; CHECK: ## %bb.0:
81+
; CHECK-NEXT: ## InlineAsm Start
6782
; CHECK-NEXT: rolw $8, %di
83+
; CHECK-NEXT: ## InlineAsm End
6884
; CHECK-NEXT: movzwl %di, %eax
6985
; CHECK-NEXT: retq
7086
%asmtmp = tail call i16 asm "rolw $$8, ${0:w}", "=r,0,~{cc},~{dirflag},~{fpsr},~{flags}"(i16 %x) nounwind
@@ -75,7 +91,9 @@ define i32 @s32(i32 %x) nounwind {
7591
; CHECK-LABEL: s32:
7692
; CHECK: ## %bb.0:
7793
; CHECK-NEXT: movl %edi, %eax
94+
; CHECK-NEXT: ## InlineAsm Start
7895
; CHECK-NEXT: bswapl %eax
96+
; CHECK-NEXT: ## InlineAsm End
7997
; CHECK-NEXT: retq
8098
%asmtmp = tail call i32 asm "bswap $0", "=r,0,~{dirflag},~{fpsr},~{flags}"(i32 %x) nounwind
8199
ret i32 %asmtmp
@@ -85,7 +103,9 @@ define i32 @t32(i32 %x) nounwind {
85103
; CHECK-LABEL: t32:
86104
; CHECK: ## %bb.0:
87105
; CHECK-NEXT: movl %edi, %eax
106+
; CHECK-NEXT: ## InlineAsm Start
88107
; CHECK-NEXT: bswapl %eax
108+
; CHECK-NEXT: ## InlineAsm End
89109
; CHECK-NEXT: retq
90110
%asmtmp = tail call i32 asm "bswap $0", "=r,0,~{dirflag},~{flags},~{fpsr}"(i32 %x) nounwind
91111
ret i32 %asmtmp
@@ -95,7 +115,11 @@ define i32 @u32(i32 %x) nounwind {
95115
; CHECK-LABEL: u32:
96116
; CHECK: ## %bb.0:
97117
; CHECK-NEXT: movl %edi, %eax
98-
; CHECK-NEXT: bswapl %eax
118+
; CHECK-NEXT: ## InlineAsm Start
119+
; CHECK-NEXT: rorw $8, %ax
120+
; CHECK-NEXT: rorl $16, %eax
121+
; CHECK-NEXT: rorw $8, %ax
122+
; CHECK-NEXT: ## InlineAsm End
99123
; CHECK-NEXT: retq
100124
%asmtmp = tail call i32 asm "rorw $$8, ${0:w};rorl $$16, $0;rorw $$8, ${0:w}", "=r,0,~{cc},~{dirflag},~{flags},~{fpsr}"(i32 %x) nounwind
101125
ret i32 %asmtmp
@@ -105,7 +129,9 @@ define i64 @s64(i64 %x) nounwind {
105129
; CHECK-LABEL: s64:
106130
; CHECK: ## %bb.0:
107131
; CHECK-NEXT: movq %rdi, %rax
132+
; CHECK-NEXT: ## InlineAsm Start
108133
; CHECK-NEXT: bswapq %rax
134+
; CHECK-NEXT: ## InlineAsm End
109135
; CHECK-NEXT: retq
110136
%asmtmp = tail call i64 asm "bswap ${0:q}", "=r,0,~{dirflag},~{fpsr},~{flags}"(i64 %x) nounwind
111137
ret i64 %asmtmp
@@ -115,7 +141,9 @@ define i64 @t64(i64 %x) nounwind {
115141
; CHECK-LABEL: t64:
116142
; CHECK: ## %bb.0:
117143
; CHECK-NEXT: movq %rdi, %rax
144+
; CHECK-NEXT: ## InlineAsm Start
118145
; CHECK-NEXT: bswapq %rax
146+
; CHECK-NEXT: ## InlineAsm End
119147
; CHECK-NEXT: retq
120148
%asmtmp = tail call i64 asm "bswap ${0:q}", "=r,0,~{fpsr},~{dirflag},~{flags}"(i64 %x) nounwind
121149
ret i64 %asmtmp

‎llvm/test/CodeGen/X86/inline-asm-flag-clobber.ll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ define i64 @t(ptr %arg) nounwind {
1818
ret i64 0
1919
}
2020

21-
; Make sure that we translate this to the bswap intrinsic which lowers down without the
22-
; inline assembly.
23-
; CHECK-NOT: #APP
21+
; Make sure this lowers to inline assembly and is not translated to an
22+
; intrinsic.
23+
; CHECK: #APP
2424
define i32 @s(i32 %argc, ptr nocapture %argv) unnamed_addr nounwind {
2525
entry:
2626
%0 = trunc i32 %argc to i16

‎llvm/test/CodeGen/X86/pr67333.ll‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,14 @@ declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture) #0
77
define void @SHA256_Compress_Generic(ptr noundef %ctx) #1 {
88
; CHECK-LABEL: SHA256_Compress_Generic:
99
; CHECK: # %bb.0: # %entry
10-
; CHECK-NEXT: movbel 0, %eax
11-
; CHECK-NEXT: movbel 12(%rdi), %ecx
10+
; CHECK-NEXT: movl 0, %eax
11+
; CHECK-NEXT: #APP
12+
; CHECK-NEXT: bswapl %eax
13+
; CHECK-NEXT: #NO_APP
14+
; CHECK-NEXT: movl 12(%rdi), %ecx
15+
; CHECK-NEXT: #APP
16+
; CHECK-NEXT: bswapl %ecx
17+
; CHECK-NEXT: #NO_APP
1218
; CHECK-NEXT: vmovd %eax, %xmm0
1319
; CHECK-NEXT: vmovdqa {{.*#+}} xmm1 = [128,128,128,128,0,1,2,3,128,128,128,128,128,128,128,128]
1420
; CHECK-NEXT: vpshufb %xmm1, %xmm0, %xmm2

0 commit comments

Comments
 (0)