yonghong-song added a comment.
Generally looks good. One suggestion and a few nits. Thanks!
================
Comment at: clang/lib/Basic/Targets/BPF.cpp:54
+ if (Feature == "+alu32") {
+ HasAlu32 = true;
+ }
----------------
-mcpu=v3 also enables alu32. See below llvm code
```
void BPFSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
if (CPU == "probe")
CPU = sys::detail::getHostCPUNameForBPF();
if (CPU == "generic" || CPU == "v1")
return;
if (CPU == "v2") {
HasJmpExt = true;
return;
}
if (CPU == "v3") {
HasJmpExt = true;
HasJmp32 = true;
HasAlu32 = true;
return;
}
}
```
Could you add a check with setCPU() to set HasAlu32 = true if it is v3? This
way, when people directly use -mcpu=v3 in clang command line, 'w' constraint is
also supported.
================
Comment at: clang/lib/Basic/Targets/BPF.cpp:60
+}
\ No newline at end of file
----------------
empty line here.
================
Comment at: clang/test/CodeGen/bpf-inline-asm.c:6
+
+void test_generic_constraints(int var32, long var64) {
+ asm("%0 = %1"
----------------
I assume test_generic_constraints() will pass even without this patch, but it
is a good addition for clang side of bpf inline-asm support, so I think it is
okay to be included in this patch.
================
Comment at: clang/test/CodeGen/bpf-inline-asm.c:33
+}
\ No newline at end of file
----------------
empty line at the end of file.
================
Comment at: llvm/test/CodeGen/BPF/inlineasm-wreg.ll:1
+
+; RUN: llc < %s -march=bpfel -mattr=+alu32 -verify-machineinstrs | FileCheck %s
----------------
Let us remove the empty line in the top of the test file.
================
Comment at: llvm/test/CodeGen/BPF/inlineasm-wreg.ll:20
+}
\ No newline at end of file
----------------
empty line at the end of file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102118/new/
https://reviews.llvm.org/D102118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
