[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 14 18:41:34 PDT 2021


jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53
 
+void RISCVInstrInfo::getNoop(MCInst &NopInst) const {
+  if (STI.getFeatureBits()[RISCV::FeatureStdExtC])
----------------
I will forever wonder why TII didn't make it `MCInst getNoop()`...


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:247
   }
+  return false;
 }
----------------
true, surely?


================
Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3
+;; Test the function attribute "patchable-function-entry".
+; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32
+; RUN: llc -mtriple=riscv64 < %s | FileCheck %s --check-prefixes=CHECK,64
----------------
Please match the style of the other tests: 
- update_llc_test_checks.py
- RV32I/RV32IC/RV64I/RV64IC


================
Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:5-9
+;; RVC uses 2-byte nop.
+; RUN: llc -filetype=obj -mtriple=riscv64 %s -o %t.o
+; RUN: llvm-objdump -d %t.o | FileCheck %s --check-prefix=NORVC
+; RUN: llc -filetype=obj -mtriple=riscv64 -mattr=+c %s -o %tc.o
+; RUN: llvm-objdump -d %tc.o | FileCheck %s --check-prefix=RVC
----------------
I don't think this is particularly useful if we've checked the assembly output (and it stops us using update_llc_test_checks.py), we know nops get emitted correctly, that's tested in various other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98610



More information about the llvm-commits mailing list