[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

[Jakob Stoklund Olesen]

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.

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].

/jakob