[PATCH] D13869: instruction 'align' in asm blocks works incorrectly with some cases of parameters

michael zuckerman via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 21 00:54:42 PDT 2015


m_zuckerman marked 3 inline comments as done.
m_zuckerman added a comment.

In http://reviews.llvm.org/D13869#271186, @rnk wrote:

> Actually, you need to check `MCAsmInfo::getAlignmentIsInBytes()` to decide if this rewrite is or isn't necessary. Some targets use log2 alignment values, and others use power of 2 alignment values.


We don't need to check this because we are under the rule of Microsoft. This function operate only when ParsingInlineAsm is true. This is true only in Microsoft inline asm.


================
Comment at: lib/MC/MCParser/AsmParser.cpp:4512
@@ -4511,4 +4511,3 @@
 
-  Info.AsmRewrites->push_back(
-      AsmRewrite(AOK_Align, IDLoc, 5, Log2_64(IntValue)));
+  Info.AsmRewrites->push_back(AsmRewrite(AOK_Align, IDLoc, 5));
   return false;
----------------
I tried to make as small changes as possible 
But you are right :)

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4678
@@ -4678,3 +4677,3 @@
 
     unsigned AdditionalSkip = 0;
     // Rewrite expressions in $N notation.
----------------
rnk wrote:
> rnk wrote:
> > This is now always zero, so you should zap it.
> Sorry, ignore this, I think we need to keep it along with the log2 machinery I asked you to remove.
We can keep or not the log2 machinery it's Does not matter.  When we are here. we are under the rule of Microsoft. The rule of Microsoft is the value of the number. Numbers that have log2. Microsoft leaves this number as it is, and don't do anythink more.  
Also the parseMSInlineAsm called only ones from Parser::ParseMicrosoftAsmStatement. This works when we under Microsft rule. So, the value is the inserted value. We don't need the extra AdditionalSkip. 


http://reviews.llvm.org/D13869





More information about the llvm-commits mailing list