[PATCH] D122540: [X86] Fix handling of maskmovdqu in x32 differently

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:4000-4001
 let ExeDomain = SSEPackedInt, SchedRW = [SchedWriteVecMoveLS.XMM.MR] in {
-let Uses = [EDI], Predicates = [HasAVX,Not64BitMode] in
+// As there is no attribute class for 64-bit and VEX, VMASKMOVDQU and
+// VMASKMOVDQU64 would have a decode conflict. Prefer VMASKMODDQU64.
+let Uses = [EDI], Predicates = [HasAVX], isAsmParserOnly = 1 in
----------------
skan wrote:
> hvdijk wrote:
> > skan wrote:
> > > The comment here is not correct. The disassembler knows the mode 16-bit/32-bit/64-bit when decoding an instruction, and VEX prefix does change the instruction context. I believe the conflict is b/c the only difference between the encodings of VMASKMOVDQU and VMASKMOVDQU64 is a address-size prefix.
> > The comment that we originally had was
> > ```
> >   // Special case since there is no attribute class for 64-bit and VEX
> >   if (Name == "VMASKMOVDQU64") {
> >     ShouldBeEmitted = false;
> >     return;
> >   }
> > ```
> > Are you saying that this old comment was also wrong? If so, I will check in more detail what it should be. If not, I will need a bit more help in understanding how the new comment is different in meaning from the old one.
> Maybe I misunderstood the old comment here, which was added in llvm-svn: 201299. Hi @craig.topper, could you help decide if this comment needs to be updated.
> The comment that we originally had was
> ```
>   // Special case since there is no attribute class for 64-bit and VEX
>   if (Name == "VMASKMOVDQU64") {
>     ShouldBeEmitted = false;
>     return;
>   }
> ```
> Are you saying that this old comment was also wrong? If so, I will check in more detail what it should be. If not, I will need a bit more help in understanding how the new comment is different in meaning from the old one.

I suggest the following comments:


```
// VMASKMOVDQU and VMASKMOVDQU64 differ only in the
// presence of the AdSize prefix. However, the disassembler doesn't
// care about that difference in the instruction definition; it
// handles 32-bit vs. 64-bit addressing for itself based purely
// on the 0x67 prefix and the CPU mode. 
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122540