| Message ID | Pine.LNX.4.64.1105051932362.20285@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
Joseph S. Myers wrote: > On Thu, 5 May 2011, Michael Eager wrote: > >> David Edelsohn wrote: >>> On Wed, May 4, 2011 at 7:54 AM, Joseph S. Myers <joseph@codesourcery.com> >>> wrote: >>> >>>> Two options, -mcmodel= and -mfpu=, had cases that fell through to the >>>> next case without comments to indicate if this was intended. I added >>>> comments to make the semantics explicit. Given the documentation, it >>>> may well be intentional for -mcmodel= but is more doubtful for -mfpu=. >>> I doubt that either of the fall through cases was intended. >>> >>> Alan, is mcmodel suppose to set m64? >>> >>> Michael, is mfpu suppose to set mrecip? >> No. There was a break statement at the end of case OPT_mfpu which >> disappeared when OPT_mrecip was added. > > Thanks. I'll apply this patch which removes the fall through, and adds > explicit Var and Init to the mfpu= entry in rs6000.opt to avoid problems > (when building as C++, as shown by a regression tester) with > 0-initialization of the field that gets automatically generated by the > .opt machinery for any Target option not using Var. Looks good.
On Thu, May 5, 2011 at 3:34 PM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Thu, 5 May 2011, Michael Eager wrote: > >> David Edelsohn wrote: >> > On Wed, May 4, 2011 at 7:54 AM, Joseph S. Myers <joseph@codesourcery.com> >> > wrote: >> > >> > > Two options, -mcmodel= and -mfpu=, had cases that fell through to the >> > > next case without comments to indicate if this was intended. I added >> > > comments to make the semantics explicit. Given the documentation, it >> > > may well be intentional for -mcmodel= but is more doubtful for -mfpu=. >> > >> > I doubt that either of the fall through cases was intended. >> > >> > Alan, is mcmodel suppose to set m64? >> > >> > Michael, is mfpu suppose to set mrecip? >> >> No. There was a break statement at the end of case OPT_mfpu which >> disappeared when OPT_mrecip was added. > > Thanks. I'll apply this patch which removes the fall through, and adds > explicit Var and Init to the mfpu= entry in rs6000.opt to avoid problems > (when building as C++, as shown by a regression tester) with > 0-initialization of the field that gets automatically generated by the > .opt machinery for any Target option not using Var. > > 2011-05-05 Joseph Myers <joseph@codesourcery.com> > > * config/rs6000/rs6000.c (rs6000_handle_option): Don't fall > through from -mfpu= handling. > * config/rs6000/rs6000.opt (mfpu=): Use Var and Init. Okay. The entire patch looks good with the two fall through cases corrected. Thanks, David
Index: gcc/config/rs6000/rs6000.opt =================================================================== --- gcc/config/rs6000/rs6000.opt (revision 173434) +++ gcc/config/rs6000/rs6000.opt (working copy) @@ -492,7 +492,7 @@ Target RejectNegative Var(rs6000_simple_ Floating point unit does not support divide & sqrt mfpu= -Target RejectNegative Joined Enum(fpu_type_t) +Target RejectNegative Joined Enum(fpu_type_t) Var(rs6000_fpu_type) Init(FPU_NONE) -mfpu= Specify FP (sp, dp, sp-lite, dp-lite) (implies -mxilinx-fpu) Enum Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 173434) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -4480,7 +4480,7 @@ rs6000_handle_option (struct gcc_options opts_set->x_target_flags |= MASK_SOFT_FLOAT; opts->x_rs6000_single_float = opts->x_rs6000_double_float = 0; } - /* Fall through. */ + break; case OPT_mrecip: opts->x_rs6000_recip_name = (value) ? "default" : "none";