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