[PATCH][Mips] ELF header flag support for assemblers
Jack Carter
Jack.Carter at imgtec.com
Mon Nov 18 10:34:37 PST 2013
I will make the refactoring patch and check it in.
________________________________________
> From: Rafael EspĂndola [rafael.espindola at gmail.com]
> Sent: Sunday, November 17, 2013 5:07 PM
> To: Jack Carter
> Cc: llvm-commits at cs.uiuc.edu; Vladimir Medic
> Subject: Re: [PATCH][Mips] ELF header flag support for assemblers
>
> The direction in general looks good, but
>
> * Please make the first patch a refactoring only moving
> MipsTargetStreamer to its own file. That should make the following
> patches easier to read. A patch doing just that LGTM.
I will make the refactoring patch and check it in.
>
> * Try to keep tests in sync with the code. For example, a patch adding
> parseDirectiveMipsSTO should include a .s test testing that. BTW, if
> the diritive name is just .sto, the method name can be just
> parseDirectiveSTO since.
>
I will then make patches for the STO code, default ehdr code, directive code and post them.
Test cases will match patches.
If I get past those patches I will then start addressing all the AsmPrinter directive printing and matching them with AsmParser instances in a series of patches.
>
> > There are holes that need to be filled. Currently STO symbol handling is
> > only on the integrated assembler side and needs to be done on the llvm-mc
> > side as well. The MipsTargetSteamer objects are not set up for dyn_cast.
>
> What do you want the dyn_cast for? The design of the stream interface
> is to avoid having to take different actions for object and assembly
> streamers.
I got hammered by not you, but the other MC folks before for not using dyn_cast when it was not going to be ever a choice. It was recommended to avoid static_cast. We are always guessing what will be acceptable.
I am fine with static cast here. It should be all that is needed.
>
> > I
> > don't know yet how far up it isn't ready, but we will want it fixed soon.
> > There are still some integrated assembler direct object tests and they need
> > to go once we have a unified handling of the ELF output.
> >
> > Are these patches good to commit?
>
> A refactoring patch is OK as noted above. Please then post the patch
> with test cases rebased on top.
>
> > Jack
>
> Cheers,
> Rafael
More information about the llvm-commits
mailing list