Request for review (M): 7029017 Additional architecture support for c2 compiler

[Vladimir Kozlov]

What I meant is:

in your platform .ad file you asign 'true' and in others 'false':

const bool Matcher::need_masked_shift_count = true;

in matchser.hpp:

+ static const bool need_masked_shift_count;

in mulnode.cpp:

+  if (Matcher::need_masked_shift_count) {


I am concern about masking during ideal transformation. It affects all platforms 
and you will convert a negative value to positive which may produce a different 
result since it may affect some ideal transformations. I would suggest to do it 
in .ad file.

instruct sarI_reg_imm(iRegI dst, iRegI src1, immI src2) %{
   match(Set dst (RShiftI src1 src2));
   format %{ "SRA    $src1,$src2,$dst" %}
   ins_encode %{
     int shift = ($src2$$constant) & (BitsPerInt - 1);
     __ sra($src1$$Register, shift, $dst$$Register);
   %}
   ins_pipe(ialu_reg_imm);
%}


And for variable shift you can use predicate in .ad file to avoid AND:

// Version without AND
instruct sarI_reg_reg(iRegI dst, iRegI src1, iRegI src2) %{
   predicate(n->in(2)->is_shift(BitsPerInt - 1));

// Version with AND
instruct sarI_reg_reg(iRegI dst, iRegI src1, iRegI src2) %{
   predicate(!n->in(2)->is_shift(BitsPerInt - 1));
   match(Set dst (RShiftI src1 src2));

and in node.hpp

is_shift(int mask) {
   const TypeInt* t = find_int_type();
   return (t != NULL) && t->_lo >= 0 && t->_hi <= mask;
}


Here are few comments for future C2 code modification.
Don't do assignment in the condition code:

+    const TypeInt *t;
+    // avoid infinite loop of shift transformations
+    if( !(t = phase->type(in2)->isa_int()) || t->_lo < 0 || t->_hi > m ) {

Use "if (a)" spacing instead of "if( a )".

Also you should not modify dead node, it is useless:

-  if( t == Type::TOP ) return NULL;       // Right input is dead
+  if( t == Type::TOP ) return do_mask_shift(phase, BitsPerInt - 1); // Right 
input is dead


Vladimir

Roland Westrelin wrote:
> Hi Vladimir,
> 
>> Did you consider to use Matcher property instead of new flag and new 
>> ideal node
>> code? See how we do Matcher::clone_shift_expressions.
> 
> I would like the extra and operations to be optimized out when possible. 
> That won't happen if I do the work at the matcher level, right?
> 
> Roland.