[PATCH] D130239: [LoongArch] Encode LoongArch specific ELF e_flags to binary by LoongArchTargetStreamer

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp:41
+  unsigned EFlags = MCA.getELFHeaderEFlags();
+
+  switch (ABI) {
----------------
delete blank line after `EFlags`

it's fine if you think the `EFlags` is different from the previous 3 variables and inserts a blank line before.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchELFStreamer.cpp:75
+};
+} // namespace
+
----------------
`// end namespace`


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCTargetDesc.cpp:86
+  const Triple &TT = STI.getTargetTriple();
+  if (TT.isOSBinFormatELF())
+    return new LoongArchTargetELFStreamer(S, STI);
----------------
avoid the used once variable


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCTargetDesc.cpp:125
+}
+} // end anonymous namespace
+
----------------
Below you use `end namespace`.  Be consistent.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchTargetStreamer.h:17
+namespace llvm {
+
+class LoongArchTargetStreamer : public MCTargetStreamer {
----------------
delete blank line


================
Comment at: llvm/test/CodeGen/LoongArch/e_flags.ll:1
+; RUN: llc --mtriple=loongarch32 --filetype=obj < %s | llvm-readelf -h - \
+; RUN:     | FileCheck %s --check-prefix=ILP32D --match-full-lines
----------------
```
llc ... -o %t
llvm-readelf -h %t | FileCheck ...
```

We are transitioning into using temporary files for some intermediate commands when they can be useful for debugging.


================
Comment at: llvm/test/CodeGen/LoongArch/e_flags.ll:10
+
+;; LP64D: Flags: 0x3, LP64, DOUBLE-FLOAT
+;; ILP32D: Flags: 0x7, ILP32, DOUBLE-FLOAT
----------------
Use single `;` for RUN and CHECK lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130239