[patch RFA] SH: Fix build failure for sh64-*-linux-gnu

[Joern RENNECKE]

Kaz Kojima wrote:

>I'm preparing a take 4 patch following your suggestion,
>though I have a minor problem:
>
>  
>
>>This code was wrong before, but the patch makes it worse.  That should 
>>rather be:
>>
>>      if (GET_CODE (XEXP (x, 1)) == CONST_INT
>> 	  && (CONST_OK_FOR_I10 (INTVAL (XEXP (x, 1)))
>>	      || CONST_OK_FOR_J16 (INTVAL (XEXP (x, 1)))))
>>	return 1;
>>      else
>>	return 1 + rtx_costs (XEXP (x, 1), AND); 
>>    
>>
>
>I can't find rtx_costs in the trunk sources.  Is it possible
>to replace it with another function?
>  
>
Sorry.  It's called rtx_cost.

Another issue I found by looking at rtl.h is that CONST is of class 
RTX_CONST_OBJ.
Thus, if we have a nested CONST with something non-trivial inside, we 
can end up with
invalid assembler, e.g. "(label1 - label0 >> 16) & 65535" instead of
"((label1 - label0) >> 16) & 65535" .
We could avoid the issue if we made sure that there is no nested CONST, 
but I'm wondering
now if the nested const might actually be useful if there is an UNSPEC 
inside.
Usually , a the purpose of a CONST is merely to enable the compiler to
determine that an expression is constant by looking at its top level 
only, but the compiler
could also figure that out by looking deeply inside the expression.  
Additional nested
CONSTs are not useful in this case, since the optimizers can interpret 
the actual expression
in order to do simplifications.
However, for UNSPECs (and  MEM - do we do that?), the CONST adds additional
information, telling the compiler that an expression is constant that it 
couldn't tell is const
otherwise.  So, even if we should decide to eliminate all nested consts 
now, that decision
might reasonably be reversed later, and having a design that depends on 
having no nested
CONSTs would be risky.

Thus, the test if we need another pair of parentheses for a nested 
expression should be

		  if (GET_CODE (XEXP (val, 0)) == CONST
		      || GET_RTX_CLASS (GET_CODE (XEXP (val, 0))) != RTX_OBJ)

- or something even more complicated if we want to avoid unnecessary parentheses for
some CONST expressions.  At this point, the code duplication in the patched code
becomes uncomfortably large.  I think it's better to have something like:


              rtx val = XEXP (XEXP (XEXP (x, 0), 0), 0);
              rtx val2 = val;
              bool nested_expr = false;

              fputc ('(', stream);
              if (GET_CODE (val) == ASHIFTRT)
                {
                  fputc ('(', stream);
                  val2 = XEXP (val, 0);
                }
              if (GET_CODE (val2) == CONST
                  || GET_RTX_CLASS (GET_CODE (val2))) != RTX_OBJ)
                {
                  fputc ('(', stream);
                  nested_expr = true;
                }
              output_addr_const (stream, val2);
              if (nexted_expr)
                fputc (')', stream);
              if (GET_CODE (val) == ASHIFTRT)
                {
                  fputs (" >> ", stream);
                  output_addr_const (stream, XEXP (val, 1));
                  fputc (')', stream);
          	}