[llvm-commits] Potential Patch for PR1095

[Bill Wendling]

On 1/15/07, Chris Lattner <clattner at apple.com> wrote:
> On Jan 9, 2007, at 12:25 AM, Bill Wendling wrote:
> > Hi all,
>
> Hey Bill, sorry for the delay.
>
Not a prob :-)

> > I would like someone to review this patch for PR1095. This gives us
> > a "dialect" for the assembler code if we're given something like:
> >
> >       __asm__("{cntlz|cntlzw} ...
> >
> > Before, we were defaulting to the first one. But there are some
> > platforms which need the second form.
>
> This patch is very close.  Comments below:
>
> Please add a .ll file regression test to llvm/test/Regression/CodeGen/
> PowerPC.
>
Will do.

>
> --- include/llvm/Target/TargetAsmInfo.h 21 Dec 2006 21:24:35 -0000      1.18
> +++ include/llvm/Target/TargetAsmInfo.h 9 Jan 2007 08:21:25 -0000
> @@ -27,6 +27,11 @@
>     /// properties and features specific to the target.
>     class TargetAsmInfo {
>     protected:
> +    enum AsmDialect {
> +      ASM_ATT,
> +      ASM_INTEL
> +    };
> +
>
> The meaning of the various dialects vary across the targets.  This
> enum should move into the X86 backend.  The PowerPC backend should
> use PPC_OLD_MNEMONICS = 0 and PPC_NEW_MNEMONICS = 1.  Darwin should
> use new mnemonics of course.
>
Ah! This makes more sense now. I was worried that just having "ATT"
and "INTEL" was wrong.

>
> @@ -128,6 +133,11 @@
>       /// Otherwise, it emits ".align log2(N)", e.g. 3 to align to an
> 8 byte
>       /// boundary.
>       bool AlignmentIsInBytes;              // Defaults to true
> +
> +    //===--- Assembler Dialect Information
> --------------------------------===//
> +
> +    /// AssemblerDialect - Which dialect of an assembler variant to
> use.
> +    AsmDialect AssemblerDialect;         // Defaults to ASM_ATT
>
> Please make this just an 'unsigned' and group it with InlineAsmStart/
> InlineAsmEnd.  You could make the group comment be "Inline Asm
> Directives" or something.
>
Okay.

> Otherwise, the patch looks great.  Please make these changes and
> apply it.
>
Will do! It'll probably be later today/tonight.

> Thanks Bill!
>
Thanks for the review! :-)

-bw