[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
Tue Oct 27 03:15:39 PDT 2015


m_zuckerman added inline comments.

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4716-4718
@@ -4721,1 +4715,5 @@
+      OS << ".align";
+      if (!getContext().getAsmInfo()->getAlignmentIsInBytes()){
+        assert(Val < 32 && "Expcted alignment less then 2^32.");
+      }
       break;
----------------
m_zuckerman wrote:
> m_zuckerman wrote:
> > rnk wrote:
> > > Most of your changes are unecessary. All you have to do is this change:
> > >   OS << ".align";
> > >   if (getContext().getAsmInfo()->getAlignmentIsInBytes())
> > >     break;
> > >   OS << ' ';
> > 1. Actually we just need to do change from align to .align ,that all.
> > ```
> > case AOK_Align: {
> >       OS << ".align";
> > }
> > ``` 
> > 2. We don't need the extra space. 
> > 3. We can check if the val < 32. (not must because there is another if test that doing that in  parseDirectiveAlign). 
> > 4. see the table 
> > ||**INOUT**|**OUTPUT**|**Meaning**|
> > |Microsoft| N=Number|.align N|Aligns the next variable or instruction on a byte that is a multiple of number.|
> > |Darwin|N = NUMBER 1<N<32|.align N|align_expression is a power of 2 (2^N)|
> > |Other Os Is In Bytes| N=Number|.align N|Aligns the next variable or instruction on a byte that is a multiple of number.|
> > All of the targets pass the value as it is. Except ,in the past Microsoft.
> > Some of them treat this number as power of 2 jump next byte.
> > Others treat as jump to the next byte that multiple of number. and this number is have log2.
> > 
> > 
> This path was correct for Microsoft in the past.  And we can delete row number 4718 to 4720.
> 
> Actually we just need to do change from align to .align ,that all.
> ```
> case AOK_Align: {
>       OS << ".align";
>       break;
> }
> ```
> 2.We don't need the extra space .
> 3.We can check if the val < 32. (not must because there is another if test that doing that in parseDirectiveAlign so I agree with you about that).
> 
> All of the targets pass the value as it is. Except ,in the past Microsoft.
> Some of them treat this number as power of 2 jump next byte.
> Others treat as jump to the next byte that multiple of number. 
> 
> 
This is extra check. We don't must to doing that. 


http://reviews.llvm.org/D13869





More information about the llvm-commits mailing list