Skip to content

Conversation

@MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 12, 2025

... instead of silently parsing and ignoring it without leaving an error
message.

While here, remove an unreachable @plt.

MaskRay added 2 commits April 12, 2025 16:25
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner

[skip ci]
@MaskRay MaskRay requested a review from lenary April 12, 2025 23:26
@llvmbot llvmbot added backend:RISC-V llvm:mc Machine (object) code labels Apr 12, 2025
@MaskRay MaskRay requested review from svs-quic and topperc April 12, 2025 23:26
@llvmbot
Copy link
Member

llvmbot commented Apr 12, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Fangrui Song (MaskRay)

Changes

... instead of silently parsing and ignoring it without leaving an error
message.

While here, remove an unreachable @<!-- -->plt.


Full diff: https://github.com/llvm/llvm-project/pull/135509.diff

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+2-4)
  • (modified) llvm/test/MC/RISCV/function-call-invalid.s (+1)
  • (modified) llvm/test/MC/RISCV/tail-call-invalid.s (+1)
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 952587171ffce..5804706baea9b 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2070,9 +2070,6 @@ ParseStatus RISCVAsmParser::parseBareSymbol(OperandVector &Operands) {
 
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Identifier.size());
 
-  if (Identifier.consume_back("@plt"))
-    return Error(getLoc(), "'@plt' operand not valid for instruction");
-
   MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
 
   if (Sym->isVariable()) {
@@ -2120,8 +2117,9 @@ ParseStatus RISCVAsmParser::parseCallSymbol(OperandVector &Operands) {
     Lex();
     Lex();
     StringRef PLT;
+    SMLoc Loc = getLoc();
     if (getParser().parseIdentifier(PLT) || PLT != "plt")
-      return ParseStatus::Failure;
+      return Error(Loc, "@ (except the deprecated/ignored @plt) is disallowed");
   } else if (!getLexer().peekTok().is(AsmToken::EndOfStatement)) {
     // Avoid parsing the register in `call rd, foo` as a call symbol.
     return ParseStatus::NoMatch;
diff --git a/llvm/test/MC/RISCV/function-call-invalid.s b/llvm/test/MC/RISCV/function-call-invalid.s
index 2b7a85245880d..d429c4e27ba14 100644
--- a/llvm/test/MC/RISCV/function-call-invalid.s
+++ b/llvm/test/MC/RISCV/function-call-invalid.s
@@ -10,3 +10,4 @@ call %lo(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call %hi(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call %lo(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call foo, bar # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
+call foo@pls # CHECK: :[[@LINE]]:10: error: @ (except the deprecated/ignored @plt) is disallowed
diff --git a/llvm/test/MC/RISCV/tail-call-invalid.s b/llvm/test/MC/RISCV/tail-call-invalid.s
index 270d84df58ac4..14ff996b2e4b1 100644
--- a/llvm/test/MC/RISCV/tail-call-invalid.s
+++ b/llvm/test/MC/RISCV/tail-call-invalid.s
@@ -10,3 +10,4 @@ tail %hi(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %lo(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %hi(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %lo(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
+tail foo@pls # CHECK: :[[@LINE]]:10: error: @ (except the deprecated/ignored @plt) is disallowed

.
Created using spr 1.3.5-bogner
@svs-quic
Copy link
Contributor

I had pushed #135324 a couple of days ago to fix the same issue. I'm happy to land either patch.

@MaskRay
Copy link
Member Author

MaskRay commented Apr 13, 2025

I had pushed #135324 a couple of days ago to fix the same issue. I'm happy to land either patch.

Thanks! I'll land this one (wanted to make it clear @plt is a legacy thing and test call foo@3). Let me think about the AsmBackend change.

owenca and others added 2 commits April 13, 2025 11:06
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@MaskRay MaskRay changed the base branch from users/MaskRay/spr/main.riscvasmparser-reject-call-fooinvalid to main April 13, 2025 18:07
@MaskRay MaskRay merged commit c0afb77 into main Apr 13, 2025
8 of 17 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/riscvasmparser-reject-call-fooinvalid branch April 13, 2025 18:07
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 13, 2025
... instead of silently parsing and ignoring it without leaving an error
message.

While here, remove an unreachable `@plt`.

Pull Request: llvm/llvm-project#135509
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:RISC-V llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants