[llvm-commits] [llvm] r170279 - in /llvm/trunk: include/llvm/MC/ lib/CodeGen/AsmPrinter/ lib/MC/ lib/Target/Mips/MCTargetDesc/ test/CodeGen/Mips/ tools/lto/

Jim Grosbach grosbach at apple.com
Mon Dec 17 17:57:33 PST 2012 

On Dec 17, 2012, at 5:46 PM, reed kotler <rkotler at mips.com> wrote:

> On 12/17/2012 05:38 PM, Jim Grosbach wrote:
>> On Dec 17, 2012, at 5:34 PM, Reed Kotler <rkotler at mips.com> wrote:
>> 
>>> Yes. There is more to do for mips16 in the case of direct object emission for exceptions.
>>> 
>>> But we have not done even the basics of direct object emission for mips16.There are some issues in making this work and I intend to solve it by making some additions to tablegen in order to allow
>>> multiple instruction sequences in patterns. Right now I make pseudo
>>> instructions but those are not going to be easy to directly make object code from and also they interfere with optimized instruction scheduling.
>> They're trivial to make direct object code emission from. Just expand them at MC lowering time. The ARM backend does a lot of this. This is how assembly printing is supposed to work, too. There should never be a pseudo-instruction alive as an MCInst.
>> 
>> Getting good scheduling with them, however, is indeed a bit trickier. There's often RAW dependencies in the pseudo that would be handy to schedule apart from one another, but we can't as they're not exposed to the scheduler.
>> 
>> -Ji
>> 
> 
> I can do this all in the .td files?

No.

> 
> Right now it's very easy to understand some of the complex pseudos.
> 
> In a past life of the Mips port, i.e. when it just did Mips I (which for example has no conditional moves), conditional moves were done in C++ code and they were very hard to understand. I do them now in td patterns for mips16 (which also has no conditional moves) and they are easy to understand. These same conditional move patterns are even more complex in mips16 than they
> were in Mips I.
> 
> Or are you saying that I can have pseudos in the table gen and then break them apart at MC lowering time?
> 
Right. Or a few other places earlier in the process if you want, for example, the scheduler to get access to them. Just be careful about un-modelled side effects. There's some tablegen helpers for trivial ones, and the rest are done manually. ARM uses late expansion pseudos, for example, for sequences that absolutely must be scheduled together (PICADD is an obvious example).

> My plan was to slightly extend tablegen so that these pseudos can be done in the patterns as opposed to in the instruction formats.
> 

If the instructions form a chain of defs and uses, a pattern should work now. I don't know if that functionality is used much, though, so there could be rough edges. For something more complex, a pseudo is necessary. Even then, you can expand them a lot earlier so the scheduler can see them. A custom inserter is a common choice.

Having tablegen be smart enough to auto generate those expansions would, of course, be quite handy. I don't disagree with that at all.

In any case, my objection is to "those are not going to be easy to directly make object code from." They are quite easy, and the infrastructure is in place and used. There may well be better ways for some use cases, and improvements would be welcome, but to say that what's there isn't easy is grossly overstating things.

-Jim


>>> On 12/17/2012 03:56 PM, Eli Friedman wrote:
>>>> On Sat, Dec 15, 2012 at 8:00 PM, Reed Kotler <rkotler-8NJIiSa5LzA at public.gmane.org> wrote:
>>>>> Author: rkotler
>>>>> Date: Sat Dec 15 22:00:45 2012
>>>>> New Revision: 170279
>>>>> 
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=170279&view=rev
>>>>> Log:
>>>>> This patch is needed to make c++ exceptions work for mips16.
>>>>> 
>>>>> Mips16 is really a processor decoding mode (ala thumb 1) and in the same
>>>>> program, mips16 and mips32 functions can exist and can call each other.
>>>>> 
>>>>> If a jal type instruction encounters an address with the lower bit set, then
>>>>> the processor switches to mips16 mode (if it is not already in it). If the
>>>>> lower bit is not set, then it switches to mips32 mode.
>>>>> 
>>>>> The linker knows which functions are mips16 and which are mips32.
>>>>> When relocation is performed on code labels, this lower order bit is
>>>>> set if the code label is a mips16 code label.
>>>>> 
>>>>> In general this works just fine, however when creating exception handling
>>>>> tables and dwarf, there are cases where you don't want this lower order
>>>>> bit added in.
>>>>> 
>>>>> This has been traditionally distinguished in gas assembly source by using a
>>>>> different syntax for the label.
>>>>> 
>>>>> lab1:      ; this will cause the lower order bit to be added
>>>>> lab2=.     ; this will not cause the lower order bit to be added
>>>>> 
>>>>> In some cases, it does not matter because in dwarf and debug tables
>>>>> the difference of two labels is used and in that case the lower order
>>>>> bits subtract each other out.
>>>>> 
>>>>> To fix this, I have added to mcstreamer the notion of a debuglabel.
>>>>> The default is for label and debug label to be the same. So calling
>>>>> EmitLabel and EmitDebugLabel produce the same result.
>>>>> 
>>>>> For various reasons, there is only one set of labels that needs to be
>>>>> modified for the mips exceptions to work. These are the "$eh_func_beginXXX"
>>>>> labels.
>>>>> 
>>>>> Mips overrides the debug label suffix from ":" to "=." .
>>>>> 
>>>>> This initial patch fixes exceptions. More changes most likely
>>>>> will be needed to DwarfCFException to make all of this work
>>>>> for actual debugging. These changes will be to emit debug labels in some
>>>>> places where a simple label is emitted now.
>>>>> 
>>>>> Some historical discussion on this from gcc can be found at:
>>>>> http://gcc.gnu.org/ml/gcc-patches/2008-08/msg00623.html
>>>>> http://gcc.gnu.org/ml/gcc-patches/2008-11/msg01273.html
>>>>> 
>>>>> 
>>>>> Added:
>>>>>     llvm/trunk/test/CodeGen/Mips/mips16ex.ll
>>>>> Modified:
>>>>>     llvm/trunk/include/llvm/MC/MCAsmInfo.h
>>>>>     llvm/trunk/include/llvm/MC/MCELFStreamer.h
>>>>>     llvm/trunk/include/llvm/MC/MCObjectStreamer.h
>>>>>     llvm/trunk/include/llvm/MC/MCStreamer.h
>>>>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
>>>>>     llvm/trunk/lib/MC/MCAsmInfo.cpp
>>>>>     llvm/trunk/lib/MC/MCAsmStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCELFStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCMachOStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCNullStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCObjectStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCPureStreamer.cpp
>>>>>     llvm/trunk/lib/MC/MCStreamer.cpp
>>>>>     llvm/trunk/lib/MC/WinCOFFStreamer.cpp
>>>>>     llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp
>>>>>     llvm/trunk/tools/lto/LTOModule.cpp
>>>>> 
>>>>> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
>>>>> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Sat Dec 15 22:00:45 2012
>>>>> @@ -102,6 +102,9 @@
>>>>>      /// LabelSuffix - This is appended to emitted labels.
>>>>>      const char *LabelSuffix;                 // Defaults to ":"
>>>>> 
>>>>> +    /// LabelSuffix - This is appended to emitted labels.
>>>>> +    const char *DebugLabelSuffix;                 // Defaults to ":"
>>>>> +
>>>>>      /// GlobalPrefix - If this is set to a non-empty string, it is prepended
>>>>>      /// onto all global symbols.  This is often used for "_" or ".".
>>>>>      const char *GlobalPrefix;                // Defaults to ""
>>>>> @@ -426,6 +429,11 @@
>>>>>      const char *getLabelSuffix() const {
>>>>>        return LabelSuffix;
>>>>>      }
>>>>> +
>>>>> +    const char *getDebugLabelSuffix() const {
>>>>> +      return DebugLabelSuffix;
>>>>> +    }
>>>>> +
>>>>>      const char *getGlobalPrefix() const {
>>>>>        return GlobalPrefix;
>>>>>      }
>>>>> 
>>>>> Modified: llvm/trunk/include/llvm/MC/MCELFStreamer.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCELFStreamer.h?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/MC/MCELFStreamer.h (original)
>>>>> +++ llvm/trunk/include/llvm/MC/MCELFStreamer.h Sat Dec 15 22:00:45 2012
>>>>> @@ -47,6 +47,7 @@
>>>>>    virtual void InitSections();
>>>>>    virtual void ChangeSection(const MCSection *Section);
>>>>>    virtual void EmitLabel(MCSymbol *Symbol);
>>>>> +  virtual void EmitDebugLabel(MCSymbol *Symbol);
>>>>>    virtual void EmitAssemblerFlag(MCAssemblerFlag Flag);
>>>>>    virtual void EmitThumbFunc(MCSymbol *Func);
>>>>>    virtual void EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol);
>>>>> 
>>>>> Modified: llvm/trunk/include/llvm/MC/MCObjectStreamer.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCObjectStreamer.h?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/MC/MCObjectStreamer.h (original)
>>>>> +++ llvm/trunk/include/llvm/MC/MCObjectStreamer.h Sat Dec 15 22:00:45 2012
>>>>> @@ -69,6 +69,7 @@
>>>>>    /// @{
>>>>> 
>>>>>    virtual void EmitLabel(MCSymbol *Symbol);
>>>>> +  virtual void EmitDebugLabel(MCSymbol *Symbol);
>>>>>    virtual void EmitAssignment(MCSymbol *Symbol, const MCExpr *Value);
>>>>>    virtual void EmitValueImpl(const MCExpr *Value, unsigned Size,
>>>>>                               unsigned AddrSpace);
>>>>> 
>>>>> Modified: llvm/trunk/include/llvm/MC/MCStreamer.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/MC/MCStreamer.h (original)
>>>>> +++ llvm/trunk/include/llvm/MC/MCStreamer.h Sat Dec 15 22:00:45 2012
>>>>> @@ -244,6 +244,8 @@
>>>>>      /// used in an assignment.
>>>>>      virtual void EmitLabel(MCSymbol *Symbol);
>>>>> 
>>>>> +    virtual void EmitDebugLabel(MCSymbol *Symbol);
>>>>> +
>>>>>      virtual void EmitEHSymAttributes(const MCSymbol *Symbol,
>>>>>                                       MCSymbol *EHSymbol);
>>>>> 
>>>>> 
>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp (original)
>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp Sat Dec 15 22:00:45 2012
>>>>> @@ -122,8 +122,9 @@
>>>>>    const MCSymbol *Sym = TLOF.getCFIPersonalitySymbol(Per, Asm->Mang, MMI);
>>>>>    Asm->OutStreamer.EmitCFIPersonality(Sym, PerEncoding);
>>>>> 
>>>>> -  Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("eh_func_begin",
>>>>> -                                                Asm->getFunctionNumber()));
>>>>> +  Asm->OutStreamer.EmitDebugLabel
>>>>> +    (Asm->GetTempSymbol("eh_func_begin",
>>>>> +                        Asm->getFunctionNumber()));
>>>>> 
>>>>>    // Provide LSDA information.
>>>>>    if (!shouldEmitLSDA)
>>>>> 
>>>>> Modified: llvm/trunk/lib/MC/MCAsmInfo.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfo.cpp?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/MC/MCAsmInfo.cpp (original)
>>>>> +++ llvm/trunk/lib/MC/MCAsmInfo.cpp Sat Dec 15 22:00:45 2012
>>>>> @@ -37,6 +37,7 @@
>>>>>    CommentColumn = 40;
>>>>>    CommentString = "#";
>>>>>    LabelSuffix = ":";
>>>>> +  DebugLabelSuffix = ":";
>>>>>    GlobalPrefix = "";
>>>>>    PrivateGlobalPrefix = ".";
>>>>>    LinkerPrivateGlobalPrefix = "";
>>>>> 
>>>>> Modified: llvm/trunk/lib/MC/MCAsmStreamer.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/MC/MCAsmStreamer.cpp (original)
>>>>> +++ llvm/trunk/lib/MC/MCAsmStreamer.cpp Sat Dec 15 22:00:45 2012
>>>>> @@ -135,6 +135,8 @@
>>>>>    }
>>>>> 
>>>>>    virtual void EmitLabel(MCSymbol *Symbol);
>>>>> +  virtual void EmitDebugLabel(MCSymbol *Symbol);
>>>>> +
>>>>>    virtual void EmitEHSymAttributes(const MCSymbol *Symbol,
>>>>>                                     MCSymbol *EHSymbol);
>>>>>    virtual void EmitAssemblerFlag(MCAssemblerFlag Flag);
>>>>> @@ -345,6 +347,14 @@
>>>>>    EmitEOL();
>>>>>  }
>>>>> 
>>>>> +void MCAsmStreamer::EmitDebugLabel(MCSymbol *Symbol) {
>>>>> +  assert(Symbol->isUndefined() && "Cannot define a symbol twice!");
>>>>> +  MCStreamer::EmitDebugLabel(Symbol);
>>>>> +
>>>>> +  OS << *Symbol << MAI.getDebugLabelSuffix();
>>>>> +  EmitEOL();
>>>>> +}
>>>>> +
>>>>>  void MCAsmStreamer::EmitAssemblerFlag(MCAssemblerFlag Flag) {
>>>>>    switch (Flag) {
>>>>>    case MCAF_SyntaxUnified:         OS << "\t.syntax unified"; break;
>>>>> 
>>>>> Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=170279&r1=170278&r2=170279&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
>>>>> +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Sat Dec 15 22:00:45 2012
>>>>> @@ -86,6 +86,10 @@
>>>>>      MCELF::SetType(SD, ELF::STT_TLS);
>>>>>  }
>>>>> 
>>>>> +void MCELFStreamer::EmitDebugLabel(MCSymbol *Symbol) {
>>>>> +  EmitLabel(Symbol);
>>>>> +}
>>>> This doesn't look right: shouldn't we have to do something different
>>>> for both object emission as well as asm emission?
>>>> 
>>>> -Eli
>>>> 
>

More information about the llvm-commits mailing list