[PATCH] D142685: [LoongArch] Implement handling of triple-implied ABIs

[Lu Weining via Phabricator via llvm-commits]

SixWeining added a comment.

Thanks. Overall LGTM except the handling of `FeatureBits` (maybe can be handled in a seperate patch) and some nits.



================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp:24
 
+ABI computeTargetABI(const Triple &TT, StringRef ABIName) {
+  ABI ArgProvidedABI = getTargetABI(ABIName);
----------------
Shall we handle `FeatureBits` as well?


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.cpp:50-52
+      errs() << "'" << ABIName
+             << "' is not a recognized ABI for this target, ignoring and using "
+                "triple-implied ABI\n";
----------------
Missing test?


================
Comment at: llvm/test/CodeGen/LoongArch/target-abi-from-triple.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not --crash llc --mtriple=loongarch32-linux-gnusf < %s 2>&1 \
----------------
Is this test autogenerated?


================
Comment at: llvm/test/CodeGen/LoongArch/target-abi-from-triple.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: not --crash llc --mtriple=loongarch32-linux-gnusf < %s 2>&1 \
+; RUN:   | FileCheck %s --check-prefix=ILP32S
----------------
I'm not sure whether we should add tests causing crash that may slow down testing. I remeber @MaskRay once suggested me not to do so.


================
Comment at: llvm/test/CodeGen/LoongArch/target-abi-from-triple.ll:80-84
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; 32ON64: {{.*}}
+; 64ON32: {{.*}}
+; CONFLICT-ILP32D: {{.*}}
+; CONFLICT-LP64D: {{.*}}
----------------
Are these lines useful?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142685/new/

https://reviews.llvm.org/D142685