[PATCH] D88194: [X86] CET endbr enhance

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1341
+// i.g: 0xF3660F1EFA, 0xF3670F1EFA
+  if ((Imm & 0x00FFFFFF) != 0x0F1EFA)
+    return false;
----------------
Please don't mix lower-case and upper-case hexadecimal literals.




================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1344
+
+  uint8_t OptionalPrefixBytes [] = {0x26, 0x2e, 0x36, 0x3e, 0x64,
+                                    0x65, 0x66, 0x67, 0xf0, 0xf2};
----------------
`constexpr uint8_t OptionalPrefixBytes[]`

Are these prefix bytes tested?


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1347
+  int i = 24; // 24bit 0x0F1EFA has matched
+  while (i < 64) {
+    uint8_t Byte = (Imm >> i) & 0xFF;
----------------
You can use `Imm >>= i; while (Imm != 0) { ... Imm >>= 8; }`


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1375
+bool X86DAGToDAGISel::obscureEndbrOpcodeImmediate(SDNode *N) {
+  unsigned Opc = N->getMachineOpcode();
+  MachineSDNode *N0 = nullptr;
----------------
Add const if appropriate.
ditto below


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1383
+  SDLoc dl(N);
+  switch(Opc) {
+    default: break;
----------------
`switch (Opc)`

Please clang-format the patch (`git diff -U0 --no-color 'HEAD^' | llvm-project/clang/tools/clang-format/clang-format-diff.py -i -p1`)


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1470
+      default: llvm_unreachable("Unexpected opcode!");
+      case X86::ADC32ri: NewOpc = X86::ADC32rr; break;// *32ri
+      case X86::ADD32ri: NewOpc = X86::ADD32rr; break;
----------------
There is a space before `//`


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

https://reviews.llvm.org/D88194