[PATCH] D94726: [X86] Add segment and address-size override prefixes

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3263
   // xacquire <insn>      ; xacquire must be accompanied by 'lock'
-  bool isPrefix = StringSwitch<bool>(Name)
-                      .Cases("rex64", "data32", "data16", true)
-                      .Cases("xacquire", "xrelease", true)
-                      .Cases("acquire", "release", isParsingIntelSyntax())
-                      .Default(false);
+  bool isPrefix =
+      StringSwitch<bool>(Name)
----------------
Hmm, could you capitalize the first char of "isPrefix" by the way?


================
Comment at: llvm/lib/Target/X86/X86InstrSystem.td:180-182
+                      Requires<[In32BitMode]>;
+def ADDR32_PREFIX : I<0x67, PrefixByte, (outs), (ins), "addr32", []>,
+                      Requires<[In64BitMode]>;
----------------
A minor suggestion: Add tests to check the predicates "In32BitMode", "In64BitMode" for addr16/32.


================
Comment at: llvm/test/MC/X86/addr16-32.s:24
+# CHECK-NEXT: 67 65 6f                      outsl  %gs:(%esi), %dx
+# CHECK-NEXT: c3                            retq
----------------
Remove this line.


================
Comment at: llvm/test/MC/X86/segment-prefix.s:21
+# CHECK-NEXT: 36 6f                         outsl  %ss:(%rsi), %dx
+# CHECK-NEXT: c3                            retq
----------------
Remove this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94726