On Tue, Aug 12, 2014 at 2:07 AM, Matthew Fortune <matthew.fort...@imgtec.com> wrote: > Eric Christopher <echri...@gmail.com> writes: >> On Sat, Aug 9, 2014 at 12:00 PM, Matthew Fortune >> <matthew.fort...@imgtec.com> wrote: >> > Moore, Catherine <catherine_mo...@mentor.com> writes: >> >> > -----Original Message----- >> >> > From: Steve Ellcey [mailto:sell...@mips.com] >> >> > Sent: Friday, August 08, 2014 3:42 PM >> >> > To: Moore, Catherine; matthew.fort...@imgtec.com; echri...@gmail.com; >> >> > >> >> > 2014-08-08 Steve Ellcey <sell...@mips.com> >> >> > >> >> > * config/mips/mips.h (ASM_SPEC): Pass float options to assembler. >> >> > >> >> > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index >> >> > 8d7a09f..9a15287 100644 >> >> > --- a/gcc/config/mips/mips.h >> >> > +++ b/gcc/config/mips/mips.h >> >> > @@ -1187,6 +1187,8 @@ struct mips_cpu_info { %{mshared} %{mno- >> >> > shared} \ %{msym32} %{mno-sym32} \ %{mtune=*} \ >> >> > +%{mhard-float} %{msoft-float} \ >> >> > +%{msingle-float} %{mdouble-float} \ >> >> > %(subtarget_asm_spec)" >> >> > >> >> > /* Extra switches sometimes passed to the linker. */ >> >> > >> >> >> >> Hi Steve, >> >> The patch itself looks okay, but perhaps a question for Matthew. >> >> Does the fact that the assembler requires -msoft-float even if .set >> >> softfloat is present in the .s file deliberate behavior? >> > >> > The assembler requires -msoft-float if .gnu_attribute 4,3 is given. I.e. >> the >> > overall module options must match the ABI which has been specified. .set >> > directives can still be used to override the 'current' options and be >> > inconsistent with the overall module and/or .gnu_attribute setting. >> > >> >> I don't have a problem with passing along the *float* options to gas, >> but >> >> would hope that the .set options were honored as well. >> > >> > Yes they should be. >> > >> >> My short test indicated that the .set *float* options were being ignored >> if >> >> the correct command line option wasn't present. >> > >> > I'm not certain what you are describing here. Could you confirm with an >> example >> > just in case something is not working as expected? >> > >> >> I don't have one offhand, but in skimming the binutils patch I don't >> recall seeing anything that tested this combination. May have missed >> it though. >> >> That said, the patch looks ok and if you'd like to add some tests for >> .set and the command line options to binutils that'd be great as well. > > What sort of coverage do you think we need here? I did exhaustive coverage > of anything which can affect the ABI but don't think we need the same for > command-line options vs .module vs .set. >
I probably would, but that's a different review thread the the patch is already in so I'm not going to push it here :) > There were two pre-existing tests: mips-hard-float-flag and > mips-double-float-flag which pretty much test these cases I think. Is that > OK for now until we identify any other areas which need additional > coverage? Sure. -eric
