RFR (S): Improve adlc usability

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Oct 8 18:38:24 PDT 2012


Hi Goetz,

I would not call it small with 450 lines changed :)

I filed RFE 8000592: Improve adlc usability

Review:

It is not our codding style to have NULL on left side of compare. You have it in 
several files changes.


src/share/vm/adlc/archDesc.cpp
Could you move opForm.dump() under your new check?:

    if (!result_class) opForm.dump();
-  assert( result_class, "Resulting register class was not defined for operand");
+  if (NULL == result_class) {


src/share/vm/adlc/formssel.cpp:
I don't think next code will work with num_opnds() == 0. Move num_opnds() check 
before the loop:

+const char *InstructForm::unique_opnd_ident(int idx) {
+  uint i;
+  for (i = 1; (num_opnds() >= 0) && (i < (uint)num_opnds()); ++i) {
+    if (unique_opnds_idx(i) == idx) {
+      break;
+    }
+  }
+  return (_components.at(i)) ? _components.at(i)->_name : "";
+}

src/share/vm/adlc/formssel.cpp:
Why do you need separate static method use_def_effect_name()? It is called only 
from one place.

src/share/vm/adlc/formssel.cpp:
I don't get why you need special constant_offset_unchecked() for format output. 
We dump format (PrintOptoAssembly) after code is generated so all constants 
offsets should be defined. What asserts you have "in debug output"?


src/share/vm/adlc/main.cpp:
You missed -v<GLOBALS_FILE_NAME> description:

    printf(" v  specify adGlobals output file name\n");


src/share/vm/adlc/output_h.cpp
We can't accept comment with "// SAPJVM ..."

Don't do assignment inside condition:

  134     for ( _register->reset_RegDefs(); (reg_def = 
_register->iter_RegDefs()) != NULL; ) {

And I got next build failures with your changes:

src/share/vm/adlc/output_h.cpp(86) : warning C4018: '<' : signed/unsigned mismatch

"src/share/vm/adlc/output_h.cpp", line 135: Error: Conversion of 64 bit type 
value to "int" causes truncation.
"src/share/vm/adlc/output_h.cpp", line 145: Error: Conversion of 64 bit type 
value to "int" causes truncation.
2 Error(s) detected.


Regards,
Vladimir

Lindenmaier, Goetz wrote:
> Hi,
> 
>  
> 
> We did a row of smaller changes to adlc to improve its usability, e.g.
> 
> - Make adlc more stable, especially if parsing faulty .ad files.
> 
> - In some cases raise syntax errors instead of asserting.
> 
> - Write more comments into generated files and beautify format.
> 
> - Make calls to node->format() more stable.
> 
> - Clean up adlc code: warnings, dead code etc.
> 
>  
> 
> Notice that all these changes shouldn't affect the generated code in any 
> way!
> 
>  
> 
> I collected all these and put them into this webrev:
> 
>  http://cr.openjdk.java.net/~goetz/webrevs/webrev-adlc_usability/
> 
>  
> 
> We would like to contribute this to the openJDK.  Could somebody
> 
> please review the change and push it if it seems useful to you?
> 
>  
> 
> Thanks a lot,
> 
>   Goetz.
> 
>  
> 


More information about the hotspot-compiler-dev mailing list