[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen created this revision.
Herald added subscribers: llvm-commits, cfe-commits, luismarques, apazos, 
sameer.abuasal, pzheng, s.egerton, lenary, Jim, benna, psnobl, dang, jocewei, 
PkmX, rkruppe, dexonsmith, the_o, brucehoult, MartinMosbeck, rogfer01, 
steven_wu, edward-jones, zzheng, MaskRay, jrtc27, shiva0217, kito-cheng, 
niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya, inglorion, 
mehdi_amini.
Herald added projects: clang, LLVM.

1. Support ABI checking with per function target-features

if users don't specific -mattr, the default target-feature come from IR 
attribute.

2. Enable LTO for RISCV
3. Init MCOptions.ABIName via module flag metadata


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72245

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/CodeGen/riscv-metadata.c
  clang/test/Driver/gold-lto.c
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
  llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
  llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -472,6 +472,17 @@
   if (FloatABIForCalls != FloatABI::Default)
 Options.FloatABIType = FloatABIForCalls;
 
+  if (const MDString *ModuleTargetABI =
+  dyn_cast_or_null(M->getModuleFlag("target-abi"))) {
+if (Options.MCOptions.ABIName != "" &&
+Options.MCOptions.ABIName != ModuleTargetABI->getString()) {
+  WithColor::warning(errs(), argv[0])
+  << ": warning: -target-abi option != target-abi module flag(ignoring "
+ "target-abi option)\n";
+}
+Options.MCOptions.ABIName = ModuleTargetABI->getString();
+  }
+
   // Figure out where we are going to send the output.
   std::unique_ptr Out =
   GetOutputStream(TheTarget->getName(), TheTriple.getOS(), argv[0]);
Index: llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x2, single-float ABI
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT-NEXT: fcvt.s.w  fa0, a0
+; DEFAULT-NEXT: ret
+; RV32IF-ILP32F: # %bb.0:
+; RV32IF-ILP32F-NEXT: fcvt.s.w  fa0, a0
+; RV32IF-ILP32F-NEXT: ret
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32f"}
Index: llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
===
--- llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
+++ llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
@@ -2,12 +2,17 @@
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -mattr=-f -target-abi ilp32f <%s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32I-ILP32F-FAILED %s
+
+; RV32I-ILP32F-FAILED: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension
 
-; RV32IF-ILP32F: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension (ignoring target-abi)
 
 define float @foo(i32 %a) nounwind #0 {
-; RV32IF-ILP32: # %bb.0:
-; RV32IF-ILP32-NEXT: fcvt.s.w  ft0, a0
+; RV32IF-ILP32: fcvt.s.w  ft0, a0
+; RV32IF-ILP32-NEXT: fmv.x.w a0, ft0
+; RV32IF-ILP32F: fcvt.s.w fa0, a0
+; RV32IF-ILP32F-NEXT: ret
   %conv = sitofp i32 %a to float
   ret float %conv
 }
Index: llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
===
--- llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
+++ llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
@@ -202,6 +202,8 @@
 ABI computeTargetABI(const Triple &TT, FeatureBitset FeatureBits,
  StringRef ABIName);
 
+ABI getTargetABI(StringRef ABIName);
+
 // Returns the register used to hold the stack pointer after realignment.
 Register

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 236296.

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

https://reviews.llvm.org/D72245

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/CodeGen/riscv-metadata.c
  clang/test/Driver/gold-lto.c
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
  llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
  llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -456,6 +456,18 @@
   Options.MCOptions.IASSearchPaths = IncludeDirs;
   Options.MCOptions.SplitDwarfFile = SplitDwarfFile;
 
+  if (const MDString *ModuleTargetABI =
+  dyn_cast_or_null(M->getModuleFlag("target-abi"))) {
+if (Options.MCOptions.ABIName != "" &&
+Options.MCOptions.ABIName != ModuleTargetABI->getString()) {
+  WithColor::warning(errs(), argv[0])
+  << ": warning: -target-abi option != target-abi module flag(ignoring "
+ "target-abi option)\n";
+}
+Options.MCOptions.ABIName = ModuleTargetABI->getString();
+  }
+
+
   std::unique_ptr Target(TheTarget->createTargetMachine(
   TheTriple.getTriple(), CPUStr, FeaturesStr, Options, getRelocModel(),
   getCodeModel(), OLvl));
Index: llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x2, single-float ABI
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT-NEXT: fcvt.s.w  fa0, a0
+; DEFAULT-NEXT: ret
+; RV32IF-ILP32F: # %bb.0:
+; RV32IF-ILP32F-NEXT: fcvt.s.w  fa0, a0
+; RV32IF-ILP32F-NEXT: ret
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32f"}
Index: llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
===
--- llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
+++ llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
@@ -2,12 +2,17 @@
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -mattr=-f -target-abi ilp32f <%s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32I-ILP32F-FAILED %s
+
+; RV32I-ILP32F-FAILED: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension
 
-; RV32IF-ILP32F: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension (ignoring target-abi)
 
 define float @foo(i32 %a) nounwind #0 {
-; RV32IF-ILP32: # %bb.0:
-; RV32IF-ILP32-NEXT: fcvt.s.w  ft0, a0
+; RV32IF-ILP32: fcvt.s.w  ft0, a0
+; RV32IF-ILP32-NEXT: fmv.x.w a0, ft0
+; RV32IF-ILP32F: fcvt.s.w fa0, a0
+; RV32IF-ILP32F-NEXT: ret
   %conv = sitofp i32 %a to float
   ret float %conv
 }
Index: llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
===
--- llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
+++ llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
@@ -202,6 +202,8 @@
 ABI computeTargetABI(const Triple &TT, FeatureBitset FeatureBits,
  StringRef ABIName);
 
+ABI getTargetABI(StringRef ABIName);
+
 // Returns the register used to hold the stack pointer after realignment.
 Register getBPReg();
 
Index: llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
===
--- llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
+++ llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
@@ -12,16 +12,7 @@
 namespace RISCVABI {
 ABI computeTargetABI(const Triple &TT, FeatureBitset FeatureBits,
  StringRef ABIName) {
-  auto TargetABI = StringSwitch(ABIName)
-   .Case("ilp32", ABI_ILP32)
-   .Case("ilp32f", ABI_ILP32F)
-   .Case("ilp32d", ABI_I

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 237084.
khchen edited the summary of this revision.

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

https://reviews.llvm.org/D72245

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/CodeGen/riscv-metadata.c
  clang/test/Driver/gold-lto.c
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.h
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/RISCV/module-target-abi.ll
  llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
  llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -455,6 +455,7 @@
   Options.MCOptions.PreserveAsmComments = PreserveComments;
   Options.MCOptions.IASSearchPaths = IncludeDirs;
   Options.MCOptions.SplitDwarfFile = SplitDwarfFile;
+  TargetMachine::initTargetOptions(*M, Options);
 
   std::unique_ptr Target(TheTarget->createTargetMachine(
   TheTriple.getTriple(), CPUStr, FeaturesStr, Options, getRelocModel(),
Index: llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x2, single-float ABI
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT-NEXT: fcvt.s.w  fa0, a0
+; DEFAULT-NEXT: ret
+; RV32IF-ILP32F: # %bb.0:
+; RV32IF-ILP32F-NEXT: fcvt.s.w  fa0, a0
+; RV32IF-ILP32F-NEXT: ret
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32f"}
Index: llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
===
--- llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
+++ llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
@@ -2,12 +2,17 @@
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -mattr=-f -target-abi ilp32f <%s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32I-ILP32F-FAILED %s
+
+; RV32I-ILP32F-FAILED: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension
 
-; RV32IF-ILP32F: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension (ignoring target-abi)
 
 define float @foo(i32 %a) nounwind #0 {
-; RV32IF-ILP32: # %bb.0:
-; RV32IF-ILP32-NEXT: fcvt.s.w  ft0, a0
+; RV32IF-ILP32: fcvt.s.w  ft0, a0
+; RV32IF-ILP32-NEXT: fmv.x.w a0, ft0
+; RV32IF-ILP32F: fcvt.s.w fa0, a0
+; RV32IF-ILP32F-NEXT: ret
   %conv = sitofp i32 %a to float
   ret float %conv
 }
Index: llvm/test/CodeGen/RISCV/module-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/module-target-abi.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32F: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x0
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT: fmv.x.w a0, ft0
+; RV32IF-ILP32: # %bb.0:
+; RV32IF-ILP32: fmv.x.w a0, ft0
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32"}
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -63,6 +63,19 @@

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Seems like this patch mixed with LTO related changes? Could you clean it up?


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 237114.
khchen added a comment.

remote LTO related code.
this PoC include D70837  patch for generate 
correct code.


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

https://reviews.llvm.org/D72245

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/riscv-metadata.c
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/RISCV/module-target-abi.ll
  llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
  llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -455,6 +455,7 @@
   Options.MCOptions.PreserveAsmComments = PreserveComments;
   Options.MCOptions.IASSearchPaths = IncludeDirs;
   Options.MCOptions.SplitDwarfFile = SplitDwarfFile;
+  TargetMachine::initTargetOptions(*M, Options);
 
   std::unique_ptr Target(TheTarget->createTargetMachine(
   TheTriple.getTriple(), CPUStr, FeaturesStr, Options, getRelocModel(),
Index: llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x2, single-float ABI
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT-NEXT: fcvt.s.w  fa0, a0
+; DEFAULT-NEXT: ret
+; RV32IF-ILP32F: # %bb.0:
+; RV32IF-ILP32F-NEXT: fcvt.s.w  fa0, a0
+; RV32IF-ILP32F-NEXT: ret
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32f"}
Index: llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
===
--- llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
+++ llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
@@ -2,12 +2,17 @@
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -mattr=-f -target-abi ilp32f <%s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32I-ILP32F-FAILED %s
+
+; RV32I-ILP32F-FAILED: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension
 
-; RV32IF-ILP32F: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension (ignoring target-abi)
 
 define float @foo(i32 %a) nounwind #0 {
-; RV32IF-ILP32: # %bb.0:
-; RV32IF-ILP32-NEXT: fcvt.s.w  ft0, a0
+; RV32IF-ILP32: fcvt.s.w  ft0, a0
+; RV32IF-ILP32-NEXT: fmv.x.w a0, ft0
+; RV32IF-ILP32F: fcvt.s.w fa0, a0
+; RV32IF-ILP32F-NEXT: ret
   %conv = sitofp i32 %a to float
   ret float %conv
 }
Index: llvm/test/CodeGen/RISCV/module-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/module-target-abi.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32F: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x0
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT: fmv.x.w a0, ft0
+; RV32IF-ILP32: # %bb.0:
+; RV32IF-ILP32: fmv.x.w a0, ft0
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32"}
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -63,6 +63,19 @@
   RESET_OPTION(NoInfsFPMath, "no-infs-fp-math");
   RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
   RESET_OPTION(NoSig

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a subscriber: efriedma.
khchen added a comment.

Hi @efriedma 
Could you please guide me and review this PoC?
or take a look at this [[ take a look at | maillist  ]] thread?
Thank you!


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

This is going to be problematic. The Conf is a reference to the Config object 
saved on the LTO class instance shared by all backend invocations (the regular 
LTO module if one exists and any ThinLTO modules). They will end up clobbering 
each other's values here - although from the assert in initTargetOptions I see 
they are required to all have the same value anyway. Still, it is not good as 
the assert may actually be missed with unlucky interference between the 
threads. The Config object here should really be marked const, let me see if I 
can change that.

You could make a copy of the Config here, but that essentially misses the 
assertion completely. 

A better way to do this would be in LTO::addModule, which is invoked serially 
to add each Module to the LTO object.

However - this misses other places that invoke createTargetMachine - should 
other places be looking at this new module flag as well? One example I can 
think of is the old LTO API (*LTOCodeGenerator.cpp files), used by linkers such 
as ld64 and some other proprietary linkers and the llvm-lto testing tool. But I 
have no idea about other invocations of createTargetMachine.

Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs some kind 
of llvm-lto2 based test.


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

tejohnson wrote:
> This is going to be problematic. The Conf is a reference to the Config object 
> saved on the LTO class instance shared by all backend invocations (the 
> regular LTO module if one exists and any ThinLTO modules). They will end up 
> clobbering each other's values here - although from the assert in 
> initTargetOptions I see they are required to all have the same value anyway. 
> Still, it is not good as the assert may actually be missed with unlucky 
> interference between the threads. The Config object here should really be 
> marked const, let me see if I can change that.
> 
> You could make a copy of the Config here, but that essentially misses the 
> assertion completely. 
> 
> A better way to do this would be in LTO::addModule, which is invoked serially 
> to add each Module to the LTO object.
> 
> However - this misses other places that invoke createTargetMachine - should 
> other places be looking at this new module flag as well? One example I can 
> think of is the old LTO API (*LTOCodeGenerator.cpp files), used by linkers 
> such as ld64 and some other proprietary linkers and the llvm-lto testing 
> tool. But I have no idea about other invocations of createTargetMachine.
> 
> Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs some kind 
> of llvm-lto2 based test.
Thank you for this feedback. 

I've been looking at how to add an overridable TargetMachine hook which is not 
dissimilar to this static function, but is overridable by TargetMachine 
subclasses. It sounds like this approach will also not work (unless the 
TargetMachine is allowed to update its (Default)Options in LTO without issue). 

I am hoping to get a patch out today for review (which does not include the 
RISC-V specific parts of this patch, and only includes a default empty 
implementation), but I imagine it will remain unsatisfactory for LTO for the 
same reasons this is.


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

lenary wrote:
> tejohnson wrote:
> > This is going to be problematic. The Conf is a reference to the Config 
> > object saved on the LTO class instance shared by all backend invocations 
> > (the regular LTO module if one exists and any ThinLTO modules). They will 
> > end up clobbering each other's values here - although from the assert in 
> > initTargetOptions I see they are required to all have the same value 
> > anyway. Still, it is not good as the assert may actually be missed with 
> > unlucky interference between the threads. The Config object here should 
> > really be marked const, let me see if I can change that.
> > 
> > You could make a copy of the Config here, but that essentially misses the 
> > assertion completely. 
> > 
> > A better way to do this would be in LTO::addModule, which is invoked 
> > serially to add each Module to the LTO object.
> > 
> > However - this misses other places that invoke createTargetMachine - should 
> > other places be looking at this new module flag as well? One example I can 
> > think of is the old LTO API (*LTOCodeGenerator.cpp files), used by linkers 
> > such as ld64 and some other proprietary linkers and the llvm-lto testing 
> > tool. But I have no idea about other invocations of createTargetMachine.
> > 
> > Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs some 
> > kind of llvm-lto2 based test.
> Thank you for this feedback. 
> 
> I've been looking at how to add an overridable TargetMachine hook which is 
> not dissimilar to this static function, but is overridable by TargetMachine 
> subclasses. It sounds like this approach will also not work (unless the 
> TargetMachine is allowed to update its (Default)Options in LTO without 
> issue). 
> 
> I am hoping to get a patch out today for review (which does not include the 
> RISC-V specific parts of this patch, and only includes a default empty 
> implementation), but I imagine it will remain unsatisfactory for LTO for the 
> same reasons this is.
Presumably you could still do the same thing I'm suggesting here - validate and 
aggregate the value across modules in LTO::addModule. Then your hook would just 
check the aggregated setting on the Config.


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

tejohnson wrote:
> lenary wrote:
> > tejohnson wrote:
> > > This is going to be problematic. The Conf is a reference to the Config 
> > > object saved on the LTO class instance shared by all backend invocations 
> > > (the regular LTO module if one exists and any ThinLTO modules). They will 
> > > end up clobbering each other's values here - although from the assert in 
> > > initTargetOptions I see they are required to all have the same value 
> > > anyway. Still, it is not good as the assert may actually be missed with 
> > > unlucky interference between the threads. The Config object here should 
> > > really be marked const, let me see if I can change that.
> > > 
> > > You could make a copy of the Config here, but that essentially misses the 
> > > assertion completely. 
> > > 
> > > A better way to do this would be in LTO::addModule, which is invoked 
> > > serially to add each Module to the LTO object.
> > > 
> > > However - this misses other places that invoke createTargetMachine - 
> > > should other places be looking at this new module flag as well? One 
> > > example I can think of is the old LTO API (*LTOCodeGenerator.cpp files), 
> > > used by linkers such as ld64 and some other proprietary linkers and the 
> > > llvm-lto testing tool. But I have no idea about other invocations of 
> > > createTargetMachine.
> > > 
> > > Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs some 
> > > kind of llvm-lto2 based test.
> > Thank you for this feedback. 
> > 
> > I've been looking at how to add an overridable TargetMachine hook which is 
> > not dissimilar to this static function, but is overridable by TargetMachine 
> > subclasses. It sounds like this approach will also not work (unless the 
> > TargetMachine is allowed to update its (Default)Options in LTO without 
> > issue). 
> > 
> > I am hoping to get a patch out today for review (which does not include the 
> > RISC-V specific parts of this patch, and only includes a default empty 
> > implementation), but I imagine it will remain unsatisfactory for LTO for 
> > the same reasons this is.
> Presumably you could still do the same thing I'm suggesting here - validate 
> and aggregate the value across modules in LTO::addModule. Then your hook 
> would just check the aggregated setting on the Config.
D72624 is the patch I have prepared, noting I haven't had time to implement the 
aggregation yet, which suggests that patch's approach is too general.


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

lenary wrote:
> tejohnson wrote:
> > lenary wrote:
> > > tejohnson wrote:
> > > > This is going to be problematic. The Conf is a reference to the Config 
> > > > object saved on the LTO class instance shared by all backend 
> > > > invocations (the regular LTO module if one exists and any ThinLTO 
> > > > modules). They will end up clobbering each other's values here - 
> > > > although from the assert in initTargetOptions I see they are required 
> > > > to all have the same value anyway. Still, it is not good as the assert 
> > > > may actually be missed with unlucky interference between the threads. 
> > > > The Config object here should really be marked const, let me see if I 
> > > > can change that.
> > > > 
> > > > You could make a copy of the Config here, but that essentially misses 
> > > > the assertion completely. 
> > > > 
> > > > A better way to do this would be in LTO::addModule, which is invoked 
> > > > serially to add each Module to the LTO object.
> > > > 
> > > > However - this misses other places that invoke createTargetMachine - 
> > > > should other places be looking at this new module flag as well? One 
> > > > example I can think of is the old LTO API (*LTOCodeGenerator.cpp 
> > > > files), used by linkers such as ld64 and some other proprietary linkers 
> > > > and the llvm-lto testing tool. But I have no idea about other 
> > > > invocations of createTargetMachine.
> > > > 
> > > > Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs 
> > > > some kind of llvm-lto2 based test.
> > > Thank you for this feedback. 
> > > 
> > > I've been looking at how to add an overridable TargetMachine hook which 
> > > is not dissimilar to this static function, but is overridable by 
> > > TargetMachine subclasses. It sounds like this approach will also not work 
> > > (unless the TargetMachine is allowed to update its (Default)Options in 
> > > LTO without issue). 
> > > 
> > > I am hoping to get a patch out today for review (which does not include 
> > > the RISC-V specific parts of this patch, and only includes a default 
> > > empty implementation), but I imagine it will remain unsatisfactory for 
> > > LTO for the same reasons this is.
> > Presumably you could still do the same thing I'm suggesting here - validate 
> > and aggregate the value across modules in LTO::addModule. Then your hook 
> > would just check the aggregated setting on the Config.
> D72624 is the patch I have prepared, noting I haven't had time to implement 
> the aggregation yet, which suggests that patch's approach is too general.
FYI I committed a change to make the Config object passed down to the backends 
a const reference in d0aad9f56e1588effa94b15804b098e6307da6b4.


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:151
 
+  TargetMachine::initTargetOptions(M, Conf.Options);
+

tejohnson wrote:
> lenary wrote:
> > tejohnson wrote:
> > > lenary wrote:
> > > > tejohnson wrote:
> > > > > This is going to be problematic. The Conf is a reference to the 
> > > > > Config object saved on the LTO class instance shared by all backend 
> > > > > invocations (the regular LTO module if one exists and any ThinLTO 
> > > > > modules). They will end up clobbering each other's values here - 
> > > > > although from the assert in initTargetOptions I see they are required 
> > > > > to all have the same value anyway. Still, it is not good as the 
> > > > > assert may actually be missed with unlucky interference between the 
> > > > > threads. The Config object here should really be marked const, let me 
> > > > > see if I can change that.
> > > > > 
> > > > > You could make a copy of the Config here, but that essentially misses 
> > > > > the assertion completely. 
> > > > > 
> > > > > A better way to do this would be in LTO::addModule, which is invoked 
> > > > > serially to add each Module to the LTO object.
> > > > > 
> > > > > However - this misses other places that invoke createTargetMachine - 
> > > > > should other places be looking at this new module flag as well? One 
> > > > > example I can think of is the old LTO API (*LTOCodeGenerator.cpp 
> > > > > files), used by linkers such as ld64 and some other proprietary 
> > > > > linkers and the llvm-lto testing tool. But I have no idea about other 
> > > > > invocations of createTargetMachine.
> > > > > 
> > > > > Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs 
> > > > > some kind of llvm-lto2 based test.
> > > > Thank you for this feedback. 
> > > > 
> > > > I've been looking at how to add an overridable TargetMachine hook which 
> > > > is not dissimilar to this static function, but is overridable by 
> > > > TargetMachine subclasses. It sounds like this approach will also not 
> > > > work (unless the TargetMachine is allowed to update its 
> > > > (Default)Options in LTO without issue). 
> > > > 
> > > > I am hoping to get a patch out today for review (which does not include 
> > > > the RISC-V specific parts of this patch, and only includes a default 
> > > > empty implementation), but I imagine it will remain unsatisfactory for 
> > > > LTO for the same reasons this is.
> > > Presumably you could still do the same thing I'm suggesting here - 
> > > validate and aggregate the value across modules in LTO::addModule. Then 
> > > your hook would just check the aggregated setting on the Config.
> > D72624 is the patch I have prepared, noting I haven't had time to implement 
> > the aggregation yet, which suggests that patch's approach is too general.
> FYI I committed a change to make the Config object passed down to the 
> backends a const reference in d0aad9f56e1588effa94b15804b098e6307da6b4.
Thank you! It is useful to have this restriction explicit :)


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

https://reviews.llvm.org/D72245



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Zakk Chen via Phabricator via cfe-commits]

khchen abandoned this revision.
khchen added a comment.
Herald added subscribers: jobnoorman, luke, pcwang-thead, eopXD, VincentWu, 
ormris, vkmr, frasercrmck, arichardson.
Herald added a project: All.

I'm not working on RISC-V now and please reference 
https://reviews.llvm.org/D132843#3770454 to see the follow-up work.


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

https://reviews.llvm.org/D72245

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits