[llvm-commits] XOP encoding patch

Jan Sjodin jan_sjodin at yahoo.com
Thu Dec 8 18:40:39 PST 2011


>

>Regarding the MC code emitter, my suggestion here is to factor out
>common code from EmitVEXOpcodePrefix, and have a new
>EmitXOPOpcodePrefix method which reuses the common part. IMO adding
>several tricky conditions to get around the differences between them
>isn't the way to go (and compromisses the code readability). That
>means whenever one of the VEX prefixes has a slightly different
>meaning in XOP, there should be a new TSFlags for it.


I added the needed TSFlags, since the rest are exactly like VEX, so that should be no problem.

I think the ugliest bit in the patch is the one that bypasses the switch:

+
+  if(XOP) {
+    // If it is an XOP instruction it uses the VEX_5M to encode
+    // if an immediate byte is used.
+    VEX_5M = XOP_IMM ? 0x08 : 0x09;
+  } else {
+    switch (TSFlags & X86II::Op0Mask) {

Instead of doing this I could add XOP8 and XOP9 and include those cases in the switch,

so the patch would become:

   case X86II::XD:  // F2 0F
     VEX_PP = 0x3;
-    break;
+   break;
+ case X86II:XOP8:
+     VEX_5M = 0x08;
+   break; 
+ case X86II:XOP9:
+     VEX_5M = 0x09;
+   break; 


That way the code would follow the same flow. The rest of the changes are quite minimal and local.
Factoring out all the common parts into functions might not improve readability, but I could give
it a try if you feel that the XOP8/9 solution is not sufficient. 

  The other option would be to eliminate any sharing (by duplicating code), which makes sense if
one considers the sharing to be false, meaning the representation happens to be the same but
but they are really separate things.


- Jan





More information about the llvm-commits mailing list