On Tue, Oct 9, 2012 at 7:37 PM, Michael Meissner
<meiss...@linux.vnet.ibm.com> wrote:
> No before I go an redo the main part of patch #2, I have a question, which
> people prefer.
>
> The current code has sequences of:
>
> target_flags |= MASK_FOO; /* set -mfoo */
> if ((target_flags_explicit & MASK_FOO)) ... /* Did user do -mfoo or
> -mno-foo */
>
> I can either change the code like I did before:
>
> rs6000_isa_flags |= OPTION_MASK_FOO;
> if ((rs6000_isa_flags_explicit & OPTION_MASK_FOO)) ...
>
> Or I can add various macros to 'hide' the underlying bit manipulation, which
> would allow me to stage all of the changes into multiple patches. I tend to
> prefer the raw bit manipulation because it is less change overall, we've used
> raw bit manipulation forever. However I'm willing to add the accessor macros
> and submit multiple patches to get this checked in:
>
> #define TARGET_FOO_EXPLICIT ((target_flags_explict & MASK_FOO) != 0)
>
> #define SET_TARGET_FOO(VALUE) \
> do { \
> if (VALUE) \
> target_flags |= MASK_FOO; \
> else \
> target_flags &= ~MASK_FOO; \
> } while (0)
>
> #define SET_TARGET_FOO_EXPLICIT (target_flags_explicit |= MASK_FOO)
>
> and then once all of the raw access to target_flags and target_flags_explicit
> is done via accessors, I can finally do the change:
>
> #define TARGET_FOO_EXPLICIT \
> ((rs6000_isa_flags_explict & OPTION_MASK_FOO) != 0)
>
> #define SET_TARGET_FOO(VALUE) \
> do { \
> if (VALUE) \
> rs6000_isa_flags |= OPTION_MASK_FOO; \
> else \
> rs6000_isa_flags &= ~OPTION_MASK_FOO; \
> } while (0)
>
> #define SET_TARGET_FOO_EXPLICIT \
> (rs6000_isa_flags_explicit |= OPTION_MASK_FOO)
>
> An alternative would be to provide separate SET/CLEAR macros:
>
> #define TARGET_FOO_EXPLICIT ((target_flags_explict & MASK_FOO) != 0)
>
> #define SET_TARGET_FOO (target_flags |= MASK_FOO)
> #define CLEAR_TARGET_FOO (target_flags &= ~MASK_FOO)
>
> #define SET_TARGET_FOO_EXPLICIT (target_flags_explicit |= MASK_FOO)
Let's stick with the bit manipulation for the next step. I think it
would be cleaner to switch to accessor macros, but that will make this
patch bigger and more complicated. I don't think that switching piece
by piece helps any more. We can add macros in another step.
Thanks, David
