[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