This is an archive of the discontinued LLVM Phabricator instance.

[UpdateLLCTestChecks] Pick the correct output block when generating isel debug output checks in update_llc_test_checks.py
Needs ReviewPublic

Authored by ningxinr on Mar 31 2022, 7:53 AM.

Details

Summary

This change is a follow-up change of D119368. This change picks up the right block
from the generated isel debug output, and make the generated tests runnable.

With this change, the checks generated will pass. A lit tests of Lanai is added to
demonstrate what a passing test would look like.

This patch concludes the support of instruction selection debug output check generation.

Diff Detail

Event Timeline

ningxinr created this revision.Mar 31 2022, 7:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 7:53 AM
ningxinr requested review of this revision.Mar 31 2022, 7:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 7:53 AM

Just to check that I understand this correctly.
Is the intention behind the function prefix to make it configurable in the end, so that one can also check for Legalized selection DAG and others?

Or is it only to pick the right block, without making it configurable?
Then I think it would be easier to use check_label_format = '{} %s-LABEL: Selected selection DAG:{{.*}}%s%s:', which does not need any changes in common.py

llvm/utils/UpdateTestChecks/common.py
844–846

This might be easier to read?

func_prefix = func_dict[checkprefix][func_name].func_prefix
if func_prefix:
  output_lines.append(check_label_format % (checkprefix, func_prefix + '{{.*}}', func_name, args_and_sig))
ningxinr updated this revision to Diff 420502.Apr 5 2022, 7:29 AM
ningxinr edited the summary of this revision. (Show Details)

Just to check that I understand this correctly.
Is the intention behind the function prefix to make it configurable in the end, so that one can also check for Legalized selection DAG and others?

Or is it only to pick the right block, without making it configurable?
Then I think it would be easier to use check_label_format = '{} %s-LABEL: Selected selection DAG:{{.*}}%s%s:', which does not need any changes in common.py

You were right, this is much easier for my purpose. For this use case the function prefix is an invariant of function name and FileCheck prefixes, so we can just add it to the check_label_format. I suppose we could always change this later when the need of making it configurable rises.

Thank you, Sebastian! :)

ningxinr updated this revision to Diff 420508.Apr 5 2022, 7:41 AM
ningxinr retitled this revision from [UpdateLLCTestChecks] Add function prefix to make generated isel debug output checks runnable in update_llc_test_checks.py to [UpdateLLCTestChecks] Pick the correct output block when generating isel debug output checks in update_llc_test_checks.py.
ningxinr edited the summary of this revision. (Show Details)

Update commit message to reflect the feedback

Flakebi added inline comments.Apr 5 2022, 8:32 AM
llvm/utils/UpdateTestChecks/isel.py
16

I guess this TODO is fixed with this patch?

18

For consistency with the checked string below, maybe use spaces here for Selected selection DAG: %bb.0?

lebedev.ri resigned from this revision.Jan 12 2023, 5:30 PM

This review may be stuck/dead, consider abandoning if no longer relevant.
Removing myself as reviewer in attempt to clean dashboard.