From 3a0b155242a43cbe8b560b2a4f9718df72e8ffa9 Mon Sep 17 00:00:00 2001 From: goetz Date: Fri, 15 Nov 2013 12:01:00 -0800 Subject: [PATCH] 8028401: PPC (part 117): Improve usability of adlc and format() functionality. Summary: Add additional, more verbose syntax checks in adlc. Fix printing constant's problem in format(). Reviewed-by: kvn --- src/share/vm/adlc/adlparse.cpp | 35 +++++++++++++++++++++++++--------- src/share/vm/adlc/formssel.cpp | 4 ++-- src/share/vm/adlc/output_c.cpp | 29 +++++++++++++++++++++++----- src/share/vm/opto/machnode.cpp | 3 +++ src/share/vm/opto/machnode.hpp | 2 ++ 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/share/vm/adlc/adlparse.cpp b/src/share/vm/adlc/adlparse.cpp index 175bd04cf..dd303a37a 100644 --- a/src/share/vm/adlc/adlparse.cpp +++ b/src/share/vm/adlc/adlparse.cpp @@ -3996,13 +3996,11 @@ void ADLParser::effect_parse(InstructForm *instr) { //------------------------------expand_parse----------------------------------- ExpandRule* ADLParser::expand_parse(InstructForm *instr) { char *ident, *ident2; - OperandForm *oper; - InstructForm *ins; NameAndList *instr_and_operands = NULL; ExpandRule *exp = new ExpandRule(); - // Expand is a block containing an ordered list of instructions, each of - // which has an ordered list of operands. + // Expand is a block containing an ordered list of operands with initializers, + // or instructions, each of which has an ordered list of operands. // Check for block delimiter skipws(); // Skip leading whitespace if ((_curchar != '%') @@ -4016,12 +4014,30 @@ ExpandRule* ADLParser::expand_parse(InstructForm *instr) { if (ident == NULL) { parse_err(SYNERR, "identifier expected at %c\n", _curchar); continue; - } // Check that you have a valid instruction + } + + // Check whether we should parse an instruction or operand. const Form *form = _globalNames[ident]; - ins = form ? form->is_instruction() : NULL; - if (ins == NULL) { + bool parse_oper = false; + bool parse_ins = false; + if (form == NULL) { + skipws(); + // Check whether this looks like an instruction specification. If so, + // just parse the instruction. The declaration of the instruction is + // not needed here. + if (_curchar == '(') parse_ins = true; + } else if (form->is_instruction()) { + parse_ins = true; + } else if (form->is_operand()) { + parse_oper = true; + } else { + parse_err(SYNERR, "instruction/operand name expected at %s\n", ident); + continue; + } + + if (parse_oper) { // This is a new operand - oper = form ? form->is_operand() : NULL; + OperandForm *oper = form->is_operand(); if (oper == NULL) { parse_err(SYNERR, "instruction/operand name expected at %s\n", ident); continue; @@ -4056,6 +4072,7 @@ ExpandRule* ADLParser::expand_parse(InstructForm *instr) { skipws(); } else { + assert(parse_ins, "sanity"); // Add instruction to list instr_and_operands = new NameAndList(ident); // Grab operands, build nameList of them, and then put into dictionary @@ -4079,7 +4096,7 @@ ExpandRule* ADLParser::expand_parse(InstructForm *instr) { parse_err(SYNERR, "operand name expected at %s\n", ident2); continue; } - oper = form2->is_operand(); + OperandForm *oper = form2->is_operand(); if (oper == NULL && !form2->is_opclass()) { parse_err(SYNERR, "operand name expected at %s\n", ident2); continue; diff --git a/src/share/vm/adlc/formssel.cpp b/src/share/vm/adlc/formssel.cpp index 2e1e341c9..ff9351242 100644 --- a/src/share/vm/adlc/formssel.cpp +++ b/src/share/vm/adlc/formssel.cpp @@ -1276,11 +1276,11 @@ void InstructForm::rep_var_format(FILE *fp, const char *rep_var) { return; } if (strcmp(rep_var, "constantoffset") == 0) { - fprintf(fp, "st->print(\"#%%d\", constant_offset());\n"); + fprintf(fp, "st->print(\"#%%d\", constant_offset_unchecked());\n"); return; } if (strcmp(rep_var, "constantaddress") == 0) { - fprintf(fp, "st->print(\"constant table base + #%%d\", constant_offset());\n"); + fprintf(fp, "st->print(\"constant table base + #%%d\", constant_offset_unchecked());\n"); return; } diff --git a/src/share/vm/adlc/output_c.cpp b/src/share/vm/adlc/output_c.cpp index 53a09d9eb..dda0afd07 100644 --- a/src/share/vm/adlc/output_c.cpp +++ b/src/share/vm/adlc/output_c.cpp @@ -1570,6 +1570,13 @@ void ArchDesc::defineExpand(FILE *fp, InstructForm *node) { new_id = expand_instr->name(); InstructForm* expand_instruction = (InstructForm*)globalAD->globalNames()[new_id]; + + if (!expand_instruction) { + globalAD->syntax_err(node->_linenum, "In %s: instruction %s used in expand not declared\n", + node->_ident, new_id); + continue; + } + if (expand_instruction->has_temps()) { globalAD->syntax_err(node->_linenum, "In %s: expand rules using instructs with TEMPs aren't supported: %s", node->_ident, new_id); @@ -1628,6 +1635,13 @@ void ArchDesc::defineExpand(FILE *fp, InstructForm *node) { // Use 'parameter' at current position in list of new instruction's formals // instead of 'opid' when looking up info internal to new_inst const char *parameter = formal_lst->iter(); + if (!parameter) { + globalAD->syntax_err(node->_linenum, "Operand %s of expand instruction %s has" + " no equivalent in new instruction %s.", + opid, node->_ident, new_inst->_ident); + assert(0, "Wrong expand"); + } + // Check for an operand which is created in the expand rule if ((exp_pos = node->_exprule->_newopers.index(opid)) != -1) { new_pos = new_inst->operand_position(parameter,Component::USE); @@ -2103,16 +2117,21 @@ public: if (strcmp(rep_var,"$reg") == 0 || reg_conversion(rep_var) != NULL) { _reg_status = LITERAL_ACCESSED; } else { + _AD.syntax_err(_encoding._linenum, + "Invalid access to literal register parameter '%s' in %s.\n", + rep_var, _encoding._name); assert( false, "invalid access to literal register parameter"); } } // literal constant parameters must be accessed as a 'constant' field - if ( _constant_status != LITERAL_NOT_SEEN ) { - assert( _constant_status == LITERAL_SEEN, "Must have seen constant literal before now"); - if( strcmp(rep_var,"$constant") == 0 ) { - _constant_status = LITERAL_ACCESSED; + if (_constant_status != LITERAL_NOT_SEEN) { + assert(_constant_status == LITERAL_SEEN, "Must have seen constant literal before now"); + if (strcmp(rep_var,"$constant") == 0) { + _constant_status = LITERAL_ACCESSED; } else { - assert( false, "invalid access to literal constant parameter"); + _AD.syntax_err(_encoding._linenum, + "Invalid access to literal constant parameter '%s' in %s.\n", + rep_var, _encoding._name); } } } // end replacement and/or subfield diff --git a/src/share/vm/opto/machnode.cpp b/src/share/vm/opto/machnode.cpp index a1bcda959..2777058c2 100644 --- a/src/share/vm/opto/machnode.cpp +++ b/src/share/vm/opto/machnode.cpp @@ -505,6 +505,9 @@ int MachConstantNode::constant_offset() { return _constant.offset(); } +int MachConstantNode::constant_offset_unchecked() const { + return _constant.offset(); +} //============================================================================= #ifndef PRODUCT diff --git a/src/share/vm/opto/machnode.hpp b/src/share/vm/opto/machnode.hpp index 44d45ca80..c3ac46a53 100644 --- a/src/share/vm/opto/machnode.hpp +++ b/src/share/vm/opto/machnode.hpp @@ -416,6 +416,8 @@ public: int constant_offset(); int constant_offset() const { return ((MachConstantNode*) this)->constant_offset(); } + // Unchecked version to avoid assertions in debug output. + int constant_offset_unchecked() const; }; //------------------------------MachUEPNode----------------------------------- -- GitLab