[PATCH] [mips] Make the MipsAsmParser capable of knowing whether PIC mode is enabled or not.

Daniel Sanders daniel.sanders at imgtec.com
Mon Nov 10 08:53:23 PST 2014


This also needs a testcase. I understand there is a follow-up patch that effectively tests this. Could you link to it?

@rafael: We've noticed something odd in the MC class hierarchy for all targets. The details are below, could you take a look?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:105-106
@@ -103,2 +104,4 @@
                        // directive.
+  bool UsePicFromOptionDir;
+  bool IsOptionDirPicEnabled;
 
----------------
Why not initialize IsOptionDirPicEnabled from 'getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_' and drop UsePicFromOptionDir?

It turns out that doing this causes crashes and while looking into it again we've noticed something strange. It seems MipsAsmParser::Parser shadows MCAsmParserExtension::Parser. As a result, calling getContext() (which returns getParser().getContext()) may crash when getParser().getContext() is fine. The difference seems to be that getContext() uses MCAsmParserExtension::getParser() and MCAsmParserExtension::Parser whereas getParser().getContext() uses MipsAsmParser::getParser() and MipsAsmParser::Parser. MipsAsmParser::Parser is initialized by the constructor but (I think) MCAsmParserExtension is not.

@rafael: All the targets seem to do the same thing. Was it intentional?

http://reviews.llvm.org/D5626






More information about the llvm-commits mailing list