Matthew Fortune <matthew.fort...@imgtec.com> writes:
> Andrew Bennett <andrew.benn...@imgtec.com> writes:
>> I have noticed that in the mips.exp dg-option handling code the isa and
>> arch_test_option_p variables are not updated after the pre-arch to arch
>> dependency handling.  This means that if this code changes the
>> architecture the post-arch dependency handling code (which relies on
>> arch_test_option_p being true) is not run to handle any extra dependencies
>> the new architecture might need.
>
> I'm not sure this is the right place to fix this, though it does seem
> subjective as we are stretching the logic a little I think.
>
> In the pre-arch options (i.e. when an arch is not explicitly requested) we
> already have code that sets -mnan-2008 when downgrading a test R6 to R5 as
> the R6 headers will be nan2008 and there is no guarantee of nan legacy headers
> existing. This is the opposite case where we upgrade a test from R5 to R6
> and R6 has to use -mnan=2008 so needs to explicitly override any command line
> option to use -mnan=legacy. I think that therefore needs adding when we set
> the arch to R6 in the pre-arch options.
>
> At the same time I think we need to add -mabs=2008 in the same place as R6
> requires ABS2008 as well. You should see that as a failure if you test with
> -mabs=legacy.
>
> I think I wrote the exact same patch as you have when I did the original R6
> tests and concluded it was not in-keeping with the structure of mips.exp.
>
> I've added Richard too since he may be able to offer a guiding hand as
> original author of most of the mips.exp code.

Yeah, I agree that this doesn't really fit the model that well,
but like you say, we're stretching the logic a bit :-).  When I wrote it,
the architectures formed a nice tree in which moving to leaf nodes only
added features.  So in the pre-r6 days:

    # Handle dependencies between the pre-arch options and the arch option.
    # This should mirror the arch and post-arch code below.
    if { !$arch_test_option_p } {

increased the architecture from the --target_board default to match
the features required by the test, whereas:

    # Handle dependencies between the arch option and the post-arch options.
    # This should mirror the arch and pre-arch code above.
    if { $arch_test_option_p } {

turned off features from the --target_board default to match a lower
architecture required by the test.  So in the pre-r6 days, all the code
in the second block was turning something off when going to a lower
architecture.  The blocks were mutually-exclusive and writing it this
way reduced the number of redundant options.  (Admittedly you could argue
that it's daft to worry about that given the kind of command lines you
tend to get from the rest of mips.exp. :-))

r6 is the first time we've had to turn something off when moving up.
-mnan and -mabs are also the first options where old architectures
support only A, higher revisions support A and B, and the newest
revision supports only B.  I think I'd prefer to acknowledge that
and have:

    # Handle dependencies between the arch option and the post-arch options.
    # This should mirror the arch and pre-arch code above.  For pre-r6
    # architectures this only needs to be done when we've moved down
    # to a lower architecture and might need to turn features off,
    # but moving up from pre-r6 to r6 can remove features too.
    if { $arch_test_option_p || ($orig_isa_rev < 6 && $isa_rev >= 6) } {

I think the existing r6->r5 case really is different: there we're
forcing a -mnan option not because the architecture needs it but
because the environment might.

Thanks,
Richard

Reply via email to