[RISCV] emit .option directive for functions with target features which differ from module default

[BeMg]

Summary

When function has different attributes from module, emit the .option <attribute> before the function body. This allows non-integrated assemblers to properly assemble the functions (which may contain instructions dependent on the extra target features).

[kito-cheng]

You need testcase I think?

[BeMg]

move riscv-func-attr-target.ll to this patch

[asb]

llvm/test/CodeGen/RISCV/riscv-func-attr-target.ll

2 ↗

(On Diff #539904)

I think you want a test function that _doesn't_ have any additional target-features, and you can then verify that .option arch isn't blindly emitted for every function (rather than just those where it differs to the module).

[reames]

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

256

Suggestion:

if (STI->hasFeature(Feature.Value) == MCSTI.hasFeature(Feature.Value))
  continue;
auto Delta = STI->hasFeature(Feature.Value) ?  RISCVOptionArchArgType::Plus : RISCVOptionArchArgType::Minus;   
NeedEmitStdOptionArgs.emplace_back(Delta, Feature.Key);

llvm/test/CodeGen/RISCV/riscv-func-attr-target.ll

1 ↗

(On Diff #539904)

Change the test name to riscv-func-target-feature.ll

[BeMg]

1. Update test
2. Use another way to select the feature need to be emitted

[craig.topper]

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

257

Please use STI->hasFeature(Feature.Value) == MCSTI.hasFeature(Feature.Value) here

265

NeedEmitStdOptionArgs can be empty here. emitDirectiveOptionArch does not handle an empty list gracefully.

[BeMg]

1. Update condition
2. Check whether NeedEmitStdOptionArgs is empty

[reames]

LGTM

[craig.topper]

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

257

Should we check this condition first? It's probably cheaper than RISCVISAInfo::isSupportedExtensionFeature

llvm/test/CodeGen/RISCV/riscv-func-target-feature.ll

2

I don't see any tests for disabling an extension?

[BeMg]

1. reorder hasFeature and isSupportedExtensionFeature
2. Add disabling extension directive test and remove some testcase

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
 STATISTIC(RISCVNumInstrsCompressed,
           "Number of RISC-V Compressed instructions emitted");
 
+namespace llvm {
+extern const SubtargetFeatureKV RISCVFeatureKV[RISCV::NumSubtargetFeatures];
+} // namespace llvm
+
 namespace {
 class RISCVAsmPrinter : public AsmPrinter {
   const RISCVSubtarget *STI;
 
 public:
   explicit RISCVAsmPrinter(TargetMachine &TM,
                            std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)) {}
public:
 
   // Wrapper needed for tblgenned pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
   void emitFunctionEntryLabel() override;
+  void emitDirectiveOptionArch();
+  bool isSameAttribute();
 
 private:
   void emitAttributes();
 
   void emitNTLHint(const MachineInstr *MI);
 
   bool lowerToMCInst(const MachineInstr *MI, MCInst &OutMI);
 };
bool RISCVAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
   if (!DispImm.isImm())
     return true;
 
   OS << DispImm.getImm() << "("
      << RISCVInstPrinter::getRegisterName(AddrReg.getReg()) << ")";
   return false;
 }
 
+void RISCVAsmPrinter::emitDirectiveOptionArch() {
+  RISCVTargetStreamer &RTS =
+      static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
+  SmallVector<RISCVOptionArchArg> NeedEmitStdOptionArgs;
+  const MCSubtargetInfo &MCSTI = *TM.getMCSubtargetInfo();
+  for (const auto &Feature : RISCVFeatureKV) {
+    if (STI->hasFeature(Feature.Value) == MCSTI.hasFeature(Feature.Value))
+      continue;
+
+    if (!llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.Key))
+      continue;
+
+    auto Delta = STI->hasFeature(Feature.Value) ? RISCVOptionArchArgType::Plus
+                                                : RISCVOptionArchArgType::Minus;
+    NeedEmitStdOptionArgs.emplace_back(Delta, Feature.Key);
+  }
+  if (!NeedEmitStdOptionArgs.empty())
+    RTS.emitDirectiveOptionArch(NeedEmitStdOptionArgs);
+}
+
+bool RISCVAsmPrinter::isSameAttribute() {
+  const MCSubtargetInfo &MCSTI = *TM.getMCSubtargetInfo();
+  return MCSTI.getFeatureBits() == STI->getFeatureBits();
+}
+
 bool RISCVAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
   STI = &MF.getSubtarget<RISCVSubtarget>();
+  RISCVTargetStreamer &RTS =
+      static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
+  if (!isSameAttribute()) {
+    RTS.emitDirectiveOptionPush();
+    emitDirectiveOptionArch();
+  }
 
   SetupMachineFunction(MF);
   emitFunctionBody();
+
+  if (!isSameAttribute())
+    RTS.emitDirectiveOptionPop();
   return false;
 }
 
 void RISCVAsmPrinter::emitStartOfAsmFile(Module &M) {
   RISCVTargetStreamer &RTS =
       static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
   if (const MDString *ModuleTargetABI =
           dyn_cast_or_null<MDString>(M.getModuleFlag("target-abi")))

llvm/test/CodeGen/RISCV/riscv-func-target-feature.ll

+; RUN: llc -mtriple=riscv64 -mcpu=sifive-u74 -verify-machineinstrs < %s | FileCheck %s
+
+; CHECK: .option push
+; CHECK-NEXT: .option arch, +v, +zve32f, +zve32x, +zve64d, +zve64f, +zve64x, +zvl128b, +zvl32b, +zvl64b
+define void @test1() "target-features"="+a,+d,+f,+m,+c,+v,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b" {
+; CHECK-LABEL: test1
+; CHECK: .option pop
+entry:
+  ret void
+}
+
+; CHECK: .option push
+; CHECK-NEXT: .option arch, +experimental-zihintntl
+define void @test2() "target-features"="+a,+d,+f,+m,+experimental-zihintntl,+zifencei" {
+; CHECK-LABEL: test2
+; CHECK: .option pop
+entry:
+  ret void
+}
+
+; CHECK: .option push
+; CHECK-NEXT: .option arch, -a, -d, -f, -m
+define void @test3() "target-features"="-a,-d,-f,-m" {
+; CHECK-LABEL: test3
+; CHECK: .option pop
+entry:
+  ret void
+}
+
+; CHECK-NOT: .option push
+define void @test4() {
+; CHECK-LABEL: test4
+; CHECK-NOT: .option pop
+entry:
+  ret void
+}