Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LoongArch] Refactor LoongArchABI::computeTargetABI #92223

Conversation

wangleiat
Copy link
Contributor

The previous logic did not consider whether the architectural features
meet the requirements of the ABI, resulting in the generation of
incorrect object files in some cases. For example:

llc -mtriple=loongarch64 -filetype=obj test/CodeGen/LoongArch/ir-instruction/fadd.ll -o t.o
llvm-readelf -h t.o

The object file indicates the ABI as lp64d, however, the generated code
is lp64s.

The new logic introduces the feature-implied ABI. When both target-abi
and triple-implied ABI are invalid, the feature-implied ABI is used.

Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented May 15, 2024

@llvm/pr-subscribers-backend-loongarch

Author: wanglei (wangleiat)

Changes

The previous logic did not consider whether the architectural features
meet the requirements of the ABI, resulting in the generation of
incorrect object files in some cases. For example:

llc -mtriple=loongarch64 -filetype=obj test/CodeGen/LoongArch/ir-instruction/fadd.ll -o t.o
llvm-readelf -h t.o

The object file indicates the ABI as lp64d, however, the generated code
is lp64s.

The new logic introduces the feature-implied ABI. When both target-abi
and triple-implied ABI are invalid, the feature-implied ABI is used.


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

6 Files Affected:

  • (modified) llvm/lib/Target/LoongArch/LoongArchSubtarget.cpp (+1-1)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp (+102-35)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h (+2-1)
  • (modified) llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp (+2-1)
  • (modified) llvm/test/CodeGen/LoongArch/e_flags.ll (+3-3)
  • (modified) llvm/test/CodeGen/LoongArch/target-abi-from-triple-edge-cases.ll (+23)
diff --git a/llvm/lib/Target/LoongArch/LoongArchSubtarget.cpp b/llvm/lib/Target/LoongArch/LoongArchSubtarget.cpp
index ffcde7dd1fa74..57babfc917f89 100644
--- a/llvm/lib/Target/LoongArch/LoongArchSubtarget.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchSubtarget.cpp
@@ -50,7 +50,7 @@ LoongArchSubtarget &LoongArchSubtarget::initializeSubtargetDependencies(
   if (!Is64Bit && HasLA64)
     report_fatal_error("Feature 64bit should be used for loongarch64 target.");
 
-  TargetABI = LoongArchABI::computeTargetABI(TT, ABIName);
+  TargetABI = LoongArchABI::computeTargetABI(TT, getFeatureBits(), ABIName);
 
   return *this;
 }
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp
index 7fd2526a57c94..a8f3b8b079c08 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "LoongArchBaseInfo.h"
+#include "LoongArchMCTargetDesc.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -52,63 +53,129 @@ static ABI checkABIStandardized(ABI Abi) {
   return Abi;
 }
 
-ABI computeTargetABI(const Triple &TT, StringRef ABIName) {
-  ABI ArgProvidedABI = getTargetABI(ABIName);
+static ABI getTripleABI(const Triple &TT) {
   bool Is64Bit = TT.isArch64Bit();
   ABI TripleABI;
-
-  // Figure out the ABI explicitly requested via the triple's environment type.
   switch (TT.getEnvironment()) {
   case llvm::Triple::EnvironmentType::GNUSF:
-    TripleABI = Is64Bit ? LoongArchABI::ABI_LP64S : LoongArchABI::ABI_ILP32S;
+    TripleABI = Is64Bit ? ABI_LP64S : ABI_ILP32S;
     break;
   case llvm::Triple::EnvironmentType::GNUF32:
-    TripleABI = Is64Bit ? LoongArchABI::ABI_LP64F : LoongArchABI::ABI_ILP32F;
+    TripleABI = Is64Bit ? ABI_LP64F : ABI_ILP32F;
     break;
-
   // Let the fallback case behave like {ILP32,LP64}D.
   case llvm::Triple::EnvironmentType::GNUF64:
   default:
-    TripleABI = Is64Bit ? LoongArchABI::ABI_LP64D : LoongArchABI::ABI_ILP32D;
+    TripleABI = Is64Bit ? ABI_LP64D : ABI_ILP32D;
     break;
   }
+  return TripleABI;
+}
+
+ABI computeTargetABI(const Triple &TT, const FeatureBitset &FeatureBits,
+                     StringRef ABIName) {
+  bool Is64Bit = TT.isArch64Bit();
+  ABI ArgProvidedABI = getTargetABI(ABIName);
+  ABI TripleABI = getTripleABI(TT);
+
+  auto GetFeatureABI = [=]() {
+    if (FeatureBits[LoongArch::FeatureBasicD])
+      return Is64Bit ? ABI_LP64D : ABI_ILP32D;
+    if (FeatureBits[LoongArch::FeatureBasicF])
+      return Is64Bit ? ABI_LP64F : ABI_ILP32F;
+    return Is64Bit ? ABI_LP64S : ABI_ILP32S;
+  };
+  auto IsValidABI = [=](ABI Abi) {
+    switch (Abi) {
+    default:
+      return false;
+    case ABI_ILP32S:
+      return !Is64Bit;
+    case ABI_ILP32F:
+      return !Is64Bit && FeatureBits[LoongArch::FeatureBasicF];
+    case ABI_ILP32D:
+      return !Is64Bit && FeatureBits[LoongArch::FeatureBasicD];
+    case ABI_LP64S:
+      return Is64Bit;
+    case ABI_LP64F:
+      return Is64Bit && FeatureBits[LoongArch::FeatureBasicF];
+    case ABI_LP64D:
+      return Is64Bit && FeatureBits[LoongArch::FeatureBasicD];
+    }
+  };
+
+  // 1. If the '-target-abi' is valid, use it.
+  if (IsValidABI(ArgProvidedABI)) {
+    if (TT.hasEnvironment() && ArgProvidedABI != TripleABI)
+      errs()
+          << "warning: triple-implied ABI conflicts with provided target-abi '"
+          << ABIName << "', using target-abi\n";
+    return checkABIStandardized(ArgProvidedABI);
+  }
+
+  // 2. If the triple-implied ABI is valid, use it.
+  if (IsValidABI(TripleABI)) {
+    // If not specifie target-abi, use the valid triple-implied ABI.
+    if (ABIName.empty())
+      return checkABIStandardized(TripleABI);
 
-  switch (ArgProvidedABI) {
-  case LoongArchABI::ABI_Unknown:
-    // Fallback to the triple-implied ABI if ABI name is not specified or
-    // invalid.
-    if (!ABIName.empty())
+    switch (ArgProvidedABI) {
+    case ABI_Unknown:
+      // Fallback to the triple-implied ABI if ABI name is specified and
+      // invalid.
       errs() << "'" << ABIName
              << "' is not a recognized ABI for this target, ignoring and using "
                 "triple-implied ABI\n";
-    return checkABIStandardized(TripleABI);
-
-  case LoongArchABI::ABI_ILP32S:
-  case LoongArchABI::ABI_ILP32F:
-  case LoongArchABI::ABI_ILP32D:
-    if (Is64Bit) {
-      errs() << "32-bit ABIs are not supported for 64-bit targets, ignoring "
-                "target-abi and using triple-implied ABI\n";
       return checkABIStandardized(TripleABI);
+    case ABI_ILP32S:
+    case ABI_ILP32F:
+    case ABI_ILP32D:
+      if (Is64Bit) {
+        errs() << "32-bit ABIs are not supported for 64-bit targets, ignoring "
+                  "target-abi and using triple-implied ABI\n";
+        return checkABIStandardized(TripleABI);
+      }
+      break;
+    case ABI_LP64S:
+    case ABI_LP64F:
+    case ABI_LP64D:
+      if (!Is64Bit) {
+        errs() << "64-bit ABIs are not supported for 32-bit targets, ignoring "
+                  "target-abi and using triple-implied ABI\n";
+        return checkABIStandardized(TripleABI);
+      }
+      break;
     }
-    break;
 
-  case LoongArchABI::ABI_LP64S:
-  case LoongArchABI::ABI_LP64F:
-  case LoongArchABI::ABI_LP64D:
-    if (!Is64Bit) {
-      errs() << "64-bit ABIs are not supported for 32-bit targets, ignoring "
-                "target-abi and using triple-implied ABI\n";
-      return checkABIStandardized(TripleABI);
+    switch (ArgProvidedABI) {
+    case ABI_ILP32F:
+    case ABI_LP64F:
+      errs() << "'" << ABIName
+             << "' ABI can't be used for a target that doesn't support the 'F' "
+                "instruction set, ignoring target-abi and using triple-implied "
+                "ABI\n";
+      break;
+    case ABI_ILP32D:
+    case ABI_LP64D:
+      errs() << "'" << ABIName
+             << "' ABI can't be used for a target that doesn't support the 'D' "
+                "instruction set, ignoring target-abi and using triple-implied "
+                "ABI\n";
+      break;
+    default:
+      llvm_unreachable("");
     }
-    break;
+    return checkABIStandardized(TripleABI);
   }
 
-  if (!ABIName.empty() && TT.hasEnvironment() && ArgProvidedABI != TripleABI)
-    errs() << "warning: triple-implied ABI conflicts with provided target-abi '"
-           << ABIName << "', using target-abi\n";
-
-  return checkABIStandardized(ArgProvidedABI);
+  // 3. Parse the 'feature-abi', and use it.
+  if (ABIName.empty())
+    errs() << "The triple-implied ABI is invalid, ignoring triple-implied ABI "
+              "and using feature-implied ABI\n";
+  else
+    errs() << "The target-abi and triple-implied ABI are invalid, ignoring "
+              "them and using feature-implied ABI\n";
+  return checkABIStandardized(GetFeatureABI());
 }
 
 ABI getTargetABI(StringRef ABIName) {
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
index 3c3fed7d43ed9..1a12fb492a60f 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
@@ -69,7 +69,8 @@ enum ABI {
   ABI_Unknown
 };
 
-ABI computeTargetABI(const Triple &TT, StringRef ABIName);
+ABI computeTargetABI(const Triple &TT, const FeatureBitset &FeatureBits,
+                     StringRef ABIName);
 ABI getTargetABI(StringRef ABIName);
 
 // Returns the register used to hold the stack pointer after realignment.
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp
index 9e56333e5fd9b..a8a4f1ed327d3 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp
@@ -27,7 +27,8 @@ LoongArchTargetELFStreamer::LoongArchTargetELFStreamer(
   auto &MAB = static_cast<LoongArchAsmBackend &>(
       getStreamer().getAssembler().getBackend());
   setTargetABI(LoongArchABI::computeTargetABI(
-      STI.getTargetTriple(), MAB.getTargetOptions().getABIName()));
+      STI.getTargetTriple(), STI.getFeatureBits(),
+      MAB.getTargetOptions().getABIName()));
 }
 
 MCELFStreamer &LoongArchTargetELFStreamer::getStreamer() {
diff --git a/llvm/test/CodeGen/LoongArch/e_flags.ll b/llvm/test/CodeGen/LoongArch/e_flags.ll
index 25f3285bdf328..2feb9d832bca9 100644
--- a/llvm/test/CodeGen/LoongArch/e_flags.ll
+++ b/llvm/test/CodeGen/LoongArch/e_flags.ll
@@ -4,10 +4,10 @@
 ; RUN: llc --mtriple=loongarch32 --filetype=obj %s --target-abi=ilp32s -o %t-ilp32s
 ; RUN: llvm-readelf -h %t-ilp32s | FileCheck %s --check-prefixes=ILP32,ABI-S --match-full-lines
 
-; RUN: llc --mtriple=loongarch32 --filetype=obj %s --target-abi=ilp32f -o %t-ilp32f
+; RUN: llc --mtriple=loongarch32 -mattr=+f --filetype=obj %s --target-abi=ilp32f -o %t-ilp32f
 ; RUN: llvm-readelf -h %t-ilp32f | FileCheck %s --check-prefixes=ILP32,ABI-F --match-full-lines
 
-; RUN: llc --mtriple=loongarch32 --filetype=obj %s --target-abi=ilp32d -o %t-ilp32d
+; RUN: llc --mtriple=loongarch32 -mattr=+d --filetype=obj %s --target-abi=ilp32d -o %t-ilp32d
 ; RUN: llvm-readelf -h %t-ilp32d | FileCheck %s --check-prefixes=ILP32,ABI-D --match-full-lines
 
 ; RUN: llc --mtriple=loongarch64 -mattr=+d --filetype=obj %s -o %t-la64
@@ -16,7 +16,7 @@
 ; RUN: llc --mtriple=loongarch64 --filetype=obj %s --target-abi=lp64s -o %t-lp64s
 ; RUN: llvm-readelf -h %t-lp64s | FileCheck %s --check-prefixes=LP64,ABI-S --match-full-lines
 
-; RUN: llc --mtriple=loongarch64 --filetype=obj %s --target-abi=lp64f -o %t-lp64f
+; RUN: llc --mtriple=loongarch64 -mattr=+f --filetype=obj %s --target-abi=lp64f -o %t-lp64f
 ; RUN: llvm-readelf -h %t-lp64f | FileCheck %s --check-prefixes=LP64,ABI-F --match-full-lines
 
 ; RUN: llc --mtriple=loongarch64 --filetype=obj %s --mattr=+d --target-abi=lp64d -o %t-lp64d
diff --git a/llvm/test/CodeGen/LoongArch/target-abi-from-triple-edge-cases.ll b/llvm/test/CodeGen/LoongArch/target-abi-from-triple-edge-cases.ll
index 1d5ed089c69fa..457d33ca93659 100644
--- a/llvm/test/CodeGen/LoongArch/target-abi-from-triple-edge-cases.ll
+++ b/llvm/test/CodeGen/LoongArch/target-abi-from-triple-edge-cases.ll
@@ -32,6 +32,23 @@
 ; 32ON64: 32-bit ABIs are not supported for 64-bit targets, ignoring target-abi and using triple-implied ABI
 ; 64ON32: 64-bit ABIs are not supported for 32-bit targets, ignoring target-abi and using triple-implied ABI
 
+;; Check that target-abi is invalid but triple-implied ABI is valid, use triple-implied ABI
+; RUN: llc --mtriple=loongarch64-linux-gnusf --target-abi=lp64f --mattr=-f < %s 2>&1 \
+; RUN:   | FileCheck %s --check-prefixes=LP64S,LP64S-LP64F-NOF
+; RUN: llc --mtriple=loongarch64-linux-gnusf --target-abi=lp64d --mattr=-d < %s 2>&1 \
+; RUN:   | FileCheck %s --check-prefixes=LP64S,LP64S-LP64D-NOD
+
+; LP64S-LP64F-NOF: 'lp64f' ABI can't be used for a target that doesn't support the 'F' instruction set, ignoring target-abi and using triple-implied ABI
+; LP64S-LP64D-NOD: 'lp64d' ABI can't be used for a target that doesn't support the 'D' instruction set, ignoring target-abi and using triple-implied ABI
+
+;; Check that both target-abi and triple-implied ABI are invalid, use feature-implied ABI
+; RUN: llc --mtriple=loongarch64-linux-gnuf64 --target-abi=lp64f --mattr=-f < %s 2>&1 \
+; RUN:   | FileCheck %s --check-prefixes=LP64S,LP64D-LP64F-NOF
+; RUN: llc --mtriple=loongarch64 --target-abi=lp64f --mattr=-f < %s 2>&1 \
+; RUN:   | FileCheck %s --check-prefixes=LP64S,LP64D-LP64F-NOF
+
+; LP64D-LP64F-NOF: The target-abi and triple-implied ABI are invalid, ignoring them and using feature-implied ABI
+
 define float @f(float %a) {
 ; ILP32D-LABEL: f:
 ; ILP32D:       # %bb.0:
@@ -48,6 +65,9 @@ define float @f(float %a) {
 ; LP64D-NEXT:    ffint.s.w $fa1, $fa1
 ; LP64D-NEXT:    fadd.s $fa0, $fa0, $fa1
 ; LP64D-NEXT:    ret
+;
+; LP64S-LABEL: f:
+; LP64S:         bl %plt(__addsf3)
   %1 = fadd float %a, 1.0
   ret float %1
 }
@@ -69,6 +89,9 @@ define double @g(double %a) {
 ; LP64D-NEXT:    ffint.d.l $fa1, $fa1
 ; LP64D-NEXT:    fadd.d $fa0, $fa0, $fa1
 ; LP64D-NEXT:    ret
+;
+; LP64S-LABEL: g:
+; LP64S:         bl %plt(__adddf3)
   %1 = fadd double %a, 1.0
   ret double %1
 }

@wangleiat
Copy link
Contributor Author

@xen0n

Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe unify all warning messages' format into warning: blah blah? LGTM apart from the natural language usages.

Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@wangleiat wangleiat changed the base branch from users/wangleiat/spr/main.loongarch-refactor-loongarchabicomputetargetabi to main May 16, 2024 09:13
Created using spr 1.3.5-bogner
@wangleiat wangleiat merged commit 70608c2 into main May 16, 2024
3 of 4 checks passed
@wangleiat wangleiat deleted the users/wangleiat/spr/loongarch-refactor-loongarchabicomputetargetabi branch May 16, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants