patch for problem when mips16 hard float is linked as c++

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Feb 12 10:59:05 PST 2014


On 12 February 2014 13:03, reed kotler <rkotler at mips.com> wrote:
>
> This patch has been rewritten to not use output raw text in MipsAsmPrinter
> and so is being sent out to the list
> before actual commit to make sure there are no further objections.

Thanks!

> This patch has two main functions:
>
> 1) Fix a specific bug when certain conversion functions are called in a
> program compiled as mips16 with hard float and
> the program is linked as c++. There are two libraries that are reversed in
> the link order with gcc/g++ and clang/clang++ for
> mips16 in this case and the proper stubs will then not be called. These
> stubs are normally handled in the Mips16HardFloat pass
> but in this case we don't know at that time that we need to generate the
> stubs. This must all be handled later in code generation
> and we have moved this functionality to MipsAsmPrinter. When linked as C
> (gcc or clang) the proper stubs are linked in from libc.

This is strange, but as long as it is local to the Mips backend that
is probably fine. Can you share a bit more what the issue is? What
stub that needs to be generated but currently is not? When can you
find out that it is needed?

> 2) Set up the infrastructure to handle 90% of what is in the Mips16HardFloat
> pass in this new area of MipsAsmPrinter. This is a more
> logical place to handle this and we have known for some time that we needed
> to move the code later and not implement it using
> inline asm as we do now but it was not clear exactly where to do this and
> what mechanism should be used. Now it's clear to us
> how to do this and this patch contains the infrastructure to move most of
> this to MipsAsmPrinter but the actual moving will be done
> in a follow on patch. The same infrastructure is used to fix this current
> bug as described in #1. This change was requested by the list
> during the original putback of the Mips16HardFloat pass but was not
> practical for us do at that time.

The patch includes some formatting only changes like

-                 TII->get(SltOpc)).addReg(regX).addReg(regY);
+          TII->get(SltOpc)).addReg(regX).addReg(regY);

It would be easier to read without it. For the same reason, can you
clang-format it?

Looks like some functions (EmitJal for example) could be static
helpers in the .cpp file. Also, please use the current naming
convention (emitJal).

+    (".mips16.call.fp."+std::string(Symbol)

You can probably use a Twine.

I guess the main feedback is that this patch doesn't go against any of
the current llvm designs like MC or what is and is not inline
assembly, so I don't have any strong objections to it.

Cheers,
Rafael



More information about the llvm-commits mailing list