From f894e1f4847d44123fc7d6a12c5afc5fae516bca Mon Sep 17 00:00:00 2001 From: Charlie Jenkins Date: Wed, 29 Nov 2023 11:57:48 -0800 Subject: [PATCH] Fix incorrect behavior between $pseudo_op and $import The passes for parsing $pseudo_op and $import are currently swapped, resulting in incorrect behavior. Pseudo ops are defined as: "The pseudo op is only added to the overall dictionary is the dependent instruction is not present in the dictionary, else its skipped." However, this is not the case if the dependent instruction has been imported. Take the following examples: rv_pseudo ``` $pseudo_op rv_op::op pseudo rd rs1 rs2 bs 29..25=0b11000 14..12=0 6..0=0x33 ``` rv_import ``` $import rv_op::op ``` rv_op ``` op rd rs1 rs2 bs 29..25=0b11000 14..12=0 6..0=0x33 ``` If we parse rv_pseudo and rv_op, the output is the single instruction "op". This is expected, as op is the "dependent instruction" and thus the pseudo is not included. If we instead parse rv_pseudo and rv_import, the output is instead "op" and "pseudo". However, he output in this case should be the same as if the instruction was directly included. The "pseudo" instruction should not be included. This change simply swaps the pseudo_ops and import passes, causing the behavior in both of these cases to be consistent with each other. Signed-off-by: Charlie Jenkins --- parse.py | 127 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/parse.py b/parse.py index 1c41d9c0..703e983b 100755 --- a/parse.py +++ b/parse.py @@ -252,10 +252,10 @@ def create_inst_dict(file_filter, include_pseudo=False, include_pseudo_ops=[]): # update the final dict with the instruction instr_dict[name] = single_dict - # second pass if for pseudo instructions - logging.debug('Collecting pseudo instructions now') + # second pass if for imported instructions + logging.debug('Collecting imported instructions') for f in file_names: - logging.debug(f'Parsing File: {f} for pseudo_ops') + logging.debug(f'Parsing File: {f} for imported ops') with open(f) as fp: lines = (line.rstrip() for line in fp) # All lines including the blank ones @@ -266,60 +266,69 @@ def create_inst_dict(file_filter, include_pseudo=False, include_pseudo_ops=[]): # go through each line of the file for line in lines: + # if the an instruction needs to be imported then go to the + # respective file and pick the line that has the instruction. + # The variable 'line' will now point to the new line from the + # imported file - # ignore all lines not starting with $pseudo - if '$pseudo' not in line: + # ignore all lines starting with $import and $pseudo + if '$import' not in line : continue logging.debug(f' Processing line: {line}') - # use the regex pseudo_regex from constants.py to find the dependent - # extension, dependent instruction, the pseudo_op in question and - # its encoding - (ext, orig_inst, pseudo_inst, line) = pseudo_regex.findall(line)[0] - ext_file = f'{opcodes_dir}/{ext}' + (import_ext, reg_instr) = imported_regex.findall(line)[0] + import_ext_file = f'{opcodes_dir}/{import_ext}' # check if the file of the dependent extension exist. Throw error if # it doesn't - if not os.path.exists(ext_file): - ext1_file = f'{opcodes_dir}/unratified/{ext}' + if not os.path.exists(import_ext_file): + ext1_file = f'{opcodes_dir}/unratified/{import_ext}' if not os.path.exists(ext1_file): - logging.error(f'Pseudo op {pseudo_inst} in {f} depends on {ext} which is not available') + logging.error(f'Instruction {reg_instr} in {f} cannot be imported from {import_ext}') raise SystemExit(1) else: ext_file = ext1_file + else: + ext_file = import_ext_file # check if the dependent instruction exist in the dependent # extension. Else throw error. found = False for oline in open(ext_file): - if not re.findall(f'^\s*{orig_inst}\s+',oline): + if not re.findall(f'^\s*{reg_instr}\s+',oline): continue else: found = True break if not found: - logging.error(f'Orig instruction {orig_inst} not found in {ext}. Required by pseudo_op {pseudo_inst} present in {f}') + logging.error(f'imported instruction {reg_instr} not found in {ext_file}. Required by {line} present in {f}') + logging.error(f'Note: you cannot import pseudo/imported ops.') raise SystemExit(1) + # call process_enc_line to get the data about the current + # instruction + (name, single_dict) = process_enc_line(oline, f) - (name, single_dict) = process_enc_line(pseudo_inst + ' ' + line, f) - # add the pseudo_op to the dictionary only if the original - # instruction is not already in the dictionary. - if orig_inst.replace('.','_') not in instr_dict \ - or include_pseudo \ - or name in include_pseudo_ops: - - # update the final dict with the instruction - if name not in instr_dict: - instr_dict[name] = single_dict - logging.debug(f' including pseudo_ops:{name}') + # if an instruction has already been added to the filtered + # instruction dictionary throw an error saying the given + # instruction is already imported and raise SystemExit + if name in instr_dict: + var = instr_dict[name]["extension"] + if instr_dict[name]['encoding'] != single_dict['encoding']: + err_msg = f'imported instruction : {name} in ' + err_msg += f'{f.split("/")[-1]} is already ' + err_msg += f'added from {var} but each have different encodings for the same instruction' + logging.error(err_msg) + raise SystemExit(1) + instr_dict[name]['extension'].extend(single_dict['extension']) else: - logging.debug(f' Skipping pseudo_op {pseudo_inst} since original instruction {orig_inst} already selected in list') + # update the final dict with the instruction + instr_dict[name] = single_dict - # third pass if for imported instructions - logging.debug('Collecting imported instructions') + # third pass if for pseudo instructions + logging.debug('Collecting pseudo instructions now') for f in file_names: - logging.debug(f'Parsing File: {f} for imported ops') + logging.debug(f'Parsing File: {f} for pseudo_ops') with open(f) as fp: lines = (line.rstrip() for line in fp) # All lines including the blank ones @@ -330,64 +339,56 @@ def create_inst_dict(file_filter, include_pseudo=False, include_pseudo_ops=[]): # go through each line of the file for line in lines: - # if the an instruction needs to be imported then go to the - # respective file and pick the line that has the instruction. - # The variable 'line' will now point to the new line from the - # imported file - # ignore all lines starting with $import and $pseudo - if '$import' not in line : + # ignore all lines not starting with $pseudo + if '$pseudo' not in line: continue logging.debug(f' Processing line: {line}') - (import_ext, reg_instr) = imported_regex.findall(line)[0] - import_ext_file = f'{opcodes_dir}/{import_ext}' + # use the regex pseudo_regex from constants.py to find the dependent + # extension, dependent instruction, the pseudo_op in question and + # its encoding + (ext, orig_inst, pseudo_inst, line) = pseudo_regex.findall(line)[0] + ext_file = f'{opcodes_dir}/{ext}' # check if the file of the dependent extension exist. Throw error if # it doesn't - if not os.path.exists(import_ext_file): - ext1_file = f'{opcodes_dir}/unratified/{import_ext}' + if not os.path.exists(ext_file): + ext1_file = f'{opcodes_dir}/unratified/{ext}' if not os.path.exists(ext1_file): - logging.error(f'Instruction {reg_instr} in {f} cannot be imported from {import_ext}') + logging.error(f'Pseudo op {pseudo_inst} in {f} depends on {ext} which is not available') raise SystemExit(1) else: ext_file = ext1_file - else: - ext_file = import_ext_file # check if the dependent instruction exist in the dependent # extension. Else throw error. found = False for oline in open(ext_file): - if not re.findall(f'^\s*{reg_instr}\s+',oline): + if not re.findall(f'^\s*{orig_inst}\s+',oline): continue else: found = True break if not found: - logging.error(f'imported instruction {reg_instr} not found in {ext_file}. Required by {line} present in {f}') - logging.error(f'Note: you cannot import pseudo/imported ops.') + logging.error(f'Orig instruction {orig_inst} not found in {ext}. Required by pseudo_op {pseudo_inst} present in {f}') raise SystemExit(1) - # call process_enc_line to get the data about the current - # instruction - (name, single_dict) = process_enc_line(oline, f) - # if an instruction has already been added to the filtered - # instruction dictionary throw an error saying the given - # instruction is already imported and raise SystemExit - if name in instr_dict: - var = instr_dict[name]["extension"] - if instr_dict[name]['encoding'] != single_dict['encoding']: - err_msg = f'imported instruction : {name} in ' - err_msg += f'{f.split("/")[-1]} is already ' - err_msg += f'added from {var} but each have different encodings for the same instruction' - logging.error(err_msg) - raise SystemExit(1) - instr_dict[name]['extension'].extend(single_dict['extension']) - else: + (name, single_dict) = process_enc_line(pseudo_inst + ' ' + line, f) + # add the pseudo_op to the dictionary only if the original + # instruction is not already in the dictionary. + if orig_inst.replace('.','_') not in instr_dict \ + or include_pseudo \ + or name in include_pseudo_ops: + # update the final dict with the instruction - instr_dict[name] = single_dict + if name not in instr_dict: + instr_dict[name] = single_dict + logging.debug(f' including pseudo_ops:{name}') + else: + logging.debug(f' Skipping pseudo_op {pseudo_inst} since original instruction {orig_inst} already selected in list') + return instr_dict def make_priv_latex_table():