From: Eric Botcazou <ebotca...@adacore.com>
Date: Fri, 26 Oct 2012 10:57 +0200

>> @@ -1088,7 +1093,12 @@ sparc_option_override (void)
>>    if (TARGET_VIS3)
>>      target_flags |= MASK_VIS2 | MASK_VIS;
>> 
>> -  /* Don't allow -mvis, -mvis2, -mvis3, or -mfmaf if FPU is disabled.  */
>> +  /* -mcbcond implies -mvis3, -mvis2 and -mvis */
>> +  if (TARGET_CBCOND)
>> +    target_flags |= MASK_VIS3 | MASK_VIS2 | MASK_VIS;
> 
>> +@item -mcbcond
>> +@itemx -mno-cbcond
>> +@opindex mcbcond
>> +@opindex mno-cbcond
>> +With @option{-mcbcond}, GCC generates code that takes advantage of
>> +compare-and-branch instructions, as defined in the Sparc Architecture 2011.
>> +The default is @option{-mcbcond} when targeting a cpu that supports such
>> +instructions, such as niagara-4 and later.  Setting @option{-mcbcond} also
>> +sets @option{-mvis3}, @option{-mvis2}, and @option{-mvis}.
> 
> Why?  If -mcpu=niagara4 implies -mcbcond and -mvis3, I don't see the point.

Ok.  I'll make cbcond independent of the other switches.

>> +  if (TARGET_CBCOND
>> +      && GET_CODE (operands[1]) == REG
>> +      && (GET_MODE (operands[1]) == SImode
>> +      || (TARGET_ARCH64 && GET_MODE (operands[1]) == DImode))
>> +      && (GET_CODE (operands[2]) != CONST_INT
>> +      || SPARC_SIMM5_P (INTVAL (operands[2]))))
>> +    {
>> +      emit_cbcond_insn (GET_CODE (operands[0]), operands[1], operands[2],
>> operands[3]); +      return;
>> +    }
> 
> Long line.

Thanks, will fix.

>> +#ifndef HAVE_AS_SPARC4
>> +#define AS_NIAGARA4_FLAG " -xarch=v9b"
>> +#else
>> +#define AS_NIAGARA4_FLAG " -xarch=sparc4"
>> +#endif
> 
> Won't this override the AS_NIAGARA3_FLAG logic for -mcpu=niagara4?  You'll 
> have v9d for -mcpu=niagara3 but v9b for -mcpu=niagara4 if the assembler 
> doesn't support sparc4.

This is part of what we need to sort out.  What I'd really like is for
both the Solaris and generic definitions to share the switch since they
can be the same.

I'll make sure this uses the NIAGARA3 switch as the backup in the final
version I commit.

>> +;; True if we are making use of compare-and-branch instructions.
>> +;; True if we should emit a nop after a cbcond instruction
>> +(define_attr "emit_cbcond_nop" "false,true"
>> +  (symbol_ref "(emit_cbcond_nop (insn)
>> +                ? EMIT_CBCOND_NOP_TRUE : EMIT_CBCOND_NOP_FALSE)"))
>> +
>>  (define_attr "branch_type" "none,icc,fcc,reg"
>>    (const_string "none"))
> 
> There seems to be one superfluous comment line.

Thanks for pointing that out, I'll fix it up.

> Otherwise looks good.  Thanks for having introduced the -mcbcond switch, I 
> think that's perfectly appropriate for a feature like this brand new set of 
> branch instructions.

Thanks for reviewing.

Reply via email to