[llvm-commits] [llvm] r78378 - in /llvm/trunk: lib/Target/X86/AsmParser/X86AsmParser.cpp test/MC/AsmParser/x86_instructions.s utils/TableGen/AsmMatcherEmitter.cpp

[Chris Lattner]

On Aug 7, 2009, at 1:33 PM, Daniel Dunbar wrote:

>>>
>>> +//  - It may depend on the subtarget or the assembler context.
>>> Instructions
>>> +//    which are invalid for the current mode, but otherwise
>>> unambiguous (e.g.,
>>> +//    an SSE instruction in a file being assembled for i486) should
>>> be accepted
>>> +//    and rejected by the assembler front end. However, if the
>>> proper encoding
>>> +//    for an instruction is dependent on the assembler context then
>>> the matcher
>>> +//    is responsible for selecting the correct machine instruction
>>> for the
>>> +//    current mode.
>>
>> Do we really need to support "pretend i386 doesn't have sse
>> instructions"?  Do you expect a lack of this to cause a problem in
>> practice?
>
> Well, for SSE the right approach is to match the instruction, but then
> reject it in the front end. However, this approach doesn't work if the
> correct selection depends on the assembler mode. This almost certainly
> does occur in some cases, for example x86-32 vs. x86-64.

Ok, yeah, we need to be careful about mode.  Maybe the matching  
predicate can handle that...?

Worst case, we could generate two different matchers.

>>> /// FlattenVariants - Flatten an .td file assembly string by
>>> selecting the
>>> /// variant at index \arg N.
>>> static std::string FlattenVariants(const std::string &AsmString,
>>
>> Can this be shared with the asmwriter emitter?
>
> Probably. For some reason I just wanted to reimplement it. :)

You're like that! ;-)

>>> /// TokenizeAsmString - Tokenize a simplified assembly string.
>>> +static void TokenizeAsmString(const StringRef &AsmString,
>>>                               SmallVectorImpl<StringRef> &Tokens) {
>>
>> as you know, this is X86 specific.  When we bring up the next target,
>> I wonder if this info could be moved to the .td files, so that the
>> X86AsmLexer and the tblgen parser could be driven by the same point  
>> of
>> truth.
>
> This actually isn't X86 specific, it happens to tokenize ARM and PPC
> as well, relatively correctly I believe. For now I'm not worried about
> it, but I agree that we should eventually move the representation in
> the .td file to be more explicit.

Ok.

>> For warnings, how about using:
>>
>> SrcMgr.PrintMessage(CGI.TheDef->getLoc(), "whatever", "warning");
>>
>> It would be nice to add a "PrintWarning" helper like we have
>> "PrintError".
>
> Well, for now this are hidden under debug because I expect to get
> warnings for things. Once the ambiguity resolution is in place and
> other FIXMEs are resolved most of these instances turn into errors.
> IMHO we don't really want .td files to spit out warnings during a
> build, so it makes a little bit of sense to only have errors.

I mean, use:
DEBUG(SrcMgr.PrintMessage( ...

instead of
DEBUG(errs() << ...

-Chris