This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MIPS] Fix gcc.target/mips/branch-1.c for Octeon
Richard Sandiford writes:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> Nice patch, thanks.
Thanks.
> > --- 836,883 ----
> > set arch "-march=loongson2f"
> > }
> > } else {
> > ! # With ! and = the ISA value is optional.
> > ! if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)((?:![^!]+)*)$} \
> > ! $spec dummy prop relation value nocpus]
> > ! || ($value eq ""
> > ! && ($relation ne "="
> > ! || $nocpus eq ""))} {
> > ! error "Unrecognized isa specification: $spec"
> > ! }
> > ! if { $value ne "" } {
> > ! set current [mips_arch_info $arch $prop]
> > ! if { ($current < $value && ![string equal $relation "<="])
> > ! || ($current > $value && ![string equal $relation ">="])
> > ! || ([mips_have_test_option_p options "-mgp64"]
> > ! && [mips_32bit_arch_p $arch]) } {
> > ! # The current setting is out of range; it cannot
> > ! # possibly be used. Find a replacement that can.
> > ! if { [string equal $prop "isa"] } {
> > ! set arch "-mips$value"
> > ! } elseif { $value == 0 } {
> > ! set arch "-mips4"
> > } else {
> > ! if { [mips_have_option_p options "-mgp32"] } {
> > ! set arch "-mips32"
> > ! } else {
> > ! set arch "-mips64"
> > ! }
> > ! if { $value > 1 } {
> > ! append arch "r$value"
> > ! }
> > }
> > ! }
> > ! }
> > ! # Disable processor-specific extension by passing our
> > ! # generic ISA option.
> > ! if { $nocpus ne ""
> > ! && $arch eq [mips_option mips_base_options arch] } {
>
> I think this would be easier to follow as:
>
> } elseif { $nocpus ne "" } {
> # If necessary, disable processor-specific extensions
> # by passing the generic ISA option associated with ARCH.
>
> unless I'm misreading the logic.
It needs be a separate condition for this case:
isa=64!octeon // value is 64, nocpus is !octeon
. With elseif I would not handle !octeon in the first case since octeon
satisfies isa=64 so the ISA is not switched to the generic ISA.
I'll add a comment about this too.
Adam