[llvm-commits] [llvm] r163287 - in /llvm/trunk/lib/Target/Mips: CMakeLists.txt MipsAsmPrinter.cpp MipsDirectObjLower.cpp MipsDirectObjLower.h MipsMCInstLower.cpp MipsMCInstLower.h

[Carter, Jack]

Jim,

I am trying to understand what you are proposing. 

The instructions I am lowering for direct object output whether from the codegen or assembler should have this lowering described in the td files as pseudo instructions. Is that true? Even if the lowering is predicated by the value of one or more of the operands?

DSLL rd,rt,sa is transformed to DSLL32 if the operand sa is 32 or greater. If the shift amount is less than 32 no lowering is done. In no case is the lowering done if -S is used. So what is the pseudo instruction and what is the real one?

Or do I just catch this instruction in MCCodeEmitter::EncodeInstruction() and do the lowering there? 

Thanks,

Jack
________________________________________
From: Jim Grosbach [grosbach at apple.com]
Sent: Monday, September 24, 2012 12:19 PM
To: Carter, Jack
Cc: Rafael Espíndola; Akira Hatanaka; llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r163287 - in /llvm/trunk/lib/Target/Mips: CMakeLists.txt MipsAsmPrinter.cpp MipsDirectObjLower.cpp MipsDirectObjLower.h MipsMCInstLower.cpp MipsMCInstLower.h

Hi Jack,

Pseudo instructions should be explicitly marked as such. Typically, an InstAlias is sufficient. For the case where it's not, an explicit pseudo instruction that uses C++ to perform the transform is necessary. See AsmPseudoInst in the ARM backend for examples. CodeGen layer bits should use the PseudoInst mechanism and similar to get the transform.

The pseudo should never be emitted to the output streamer, including the AsmStreamer. If you want (or are required by specification) to print a pseudo instruction, explicit checks in the InstPrinter::printInstruction() perform this transform. ARM has examples of this as well.

The top-level design is that an MCInst will always, with no exceptions, represent a real encodable instruction when it's passed off to an MCStreamer. Any pseudos, be they codegen or assembler, should have been resolved earlier in the process.

-Jim


On Sep 24, 2012, at 10:51 AM, "Carter, Jack" <jcarter at mips.com> wrote:

> I am back from vacation :-(
>
> I'll look at this today.
>
> Jack
> ________________________________________
> From: Rafael Espíndola [rafael.espindola at gmail.com]
> Sent: Thursday, September 20, 2012 11:52 AM
> To: Akira Hatanaka
> Cc: Carter, Jack; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [llvm] r163287 - in /llvm/trunk/lib/Target/Mips: CMakeLists.txt MipsAsmPrinter.cpp MipsDirectObjLower.cpp MipsDirectObjLower.h MipsMCInstLower.cpp MipsMCInstLower.h
>
> On 14 September 2012 13:34, Akira Hatanaka <ahatanak at gmail.com> wrote:
>> This patch was reviewed internally at mips.
>>
>> clang actually does print the pseudo when you compile with "-S". The
>> function which does the pseudo-to-real expansion is called only if
>> OutStreamer.hasRawTextSupport() returns false:
>>
>> +    if (!OutStreamer.hasRawTextSupport()){
>>
>>
>> But I realize now that perhaps this expansion should happen in
>> MCCodeEmitter::EncodeInstruction as you suggest. The purpose of this patch
>> was to change the function to do a mcinst-to-mcinst conversion to make it
>> available to both direct object emitter and assembler. If the function is
>> called in MCCodeEmitter::EncodeInstruction, then that will take care of both
>> cases.
>
> Yes, please avoid depending on hasRawTextSupport. MC should handle
> cases like this internally. Jack, can you fix this?
>
> Cheers,
> Rafael
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits