[llvm-commits] [llvm] r163243 - in /llvm/trunk: include/llvm/CodeGen/MachineInstr.h include/llvm/InlineAsm.h lib/CodeGen/MachineInstr.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

[Chad Rosier]

On Sep 5, 2012, at 3:27 PM, Jakob Stoklund Olesen wrote:

> 
> On Sep 5, 2012, at 3:19 PM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>> 
>> On Sep 5, 2012, at 2:16 PM, Jakob Stoklund Olesen wrote:
>> 
>>> 
>>> On Sep 5, 2012, at 2:00 PM, Chad Rosier <mcrosier at apple.com> wrote:
>>> 
>>>> Author: mcrosier
>>>> Date: Wed Sep  5 16:00:58 2012
>>>> New Revision: 163243
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=163243&view=rev
>>>> Log:
>>>> [ms-inline asm] Propagate the asm dialect into the MachineInstr representation.
>>>>  // Interpretation of the MIOp_ExtraInfo bit field.
>>>>  Extra_HasSideEffects = 1,
>>>>  Extra_IsAlignStack = 2,
>>>> +    Extra_ATTDialect = 3,   // AT&T AsmDialect.
>>>> +    Extra_IntelDialect = 4, // Intel AsmDialect.
>>> 
>>>> +    if (ExtraInfo & InlineAsm::Extra_ATTDialect)
>>>> +      OS << " [attdialect]";
>>>> +    if (ExtraInfo & InlineAsm::Extra_IntelDialect)
>>>> +      OS << " [inteldialect]";
>>> 
>>> Hi Chad,
>>> 
>>> It should be enough to use one bit in ExtraInfo for the dialect, right? This implementation supports setting both at the same time.
>> 
>> How does r163257 look?
> 
> Better, but you don't seem to be using the Extra_AsmDialect enum for anything, and the shifts by 2 are a bit gross - the bitfield layout should be defined purely in the header.

Oy Vey.. *heads to coffee machine**

> Instead of:
> 
> +  return InlineAsm::AsmDialect((ExtraInfo >> 2) & 1);
> 
> Say:
> 
> +  return InlineAsm::AsmDialect((ExtraInfo & Extra_AsmDialect) != 0);
> 
> And
> 
> +  // Set the asm dialect.
> +  ExtraInfo |= IA->getDialect() * Extra_AsmDialect;
> 
> [With Extra_AsmDialect = 4, of course].
> 

Thanks for the suggestions, Jakob.  Committed revision 163263.

> /jakob
>