Page MenuHomePhabricator

[update_llc_test_checks][VE] Handle .Lfoo$local in functon regex
ClosedPublic

Authored by arichardson on Aug 8 2022, 1:17 PM.

Details

Summary

While working on https://reviews.llvm.org/D131429, I got a test diff in
one of the VE tests and running update_llc_test_checks.py deleted all the
code for that function. This updates the regex to handle this new output.

Diff Detail

Event Timeline

arichardson created this revision.Aug 8 2022, 1:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 1:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 1:29 PM
kaz7 added a comment.Aug 10 2022, 5:29 AM

It's looks like this patch itself IS removing landing pad instructions from builtin_sjlj_landingpad.ll although the explanation of this patch is saying opposite.

It's looks like this patch itself IS removing landing pad instructions from builtin_sjlj_landingpad.ll although the explanation of this patch is saying opposite.

I re-ran the update script on that file, and now it correctly detects the function boundary: Everything below .Lfunc_end0: should not have been added as a CHECK line but because the regex was wrong before it matched everything up to the next function.

kaz7 accepted this revision.Aug 10 2022, 4:35 PM

Ah, Ok. I've understood what you are trying to fix now. You are trying to correct regexp for VE in UpdateTestChecks/asm.py. I couldn't correct it, so I just hand-write this builtin_sjlj_landingpad.ll test when I created this.

Thank you!

It's looks like this patch itself IS removing landing pad instructions from builtin_sjlj_landingpad.ll although the explanation of this patch is saying opposite.

I re-ran the update script on that file, and now it correctly detects the function boundary: Everything below .Lfunc_end0: should not have been added as a CHECK line but because the regex was wrong before it matched everything up to the next function.

I was saying that removed lines including .LJIT0_0 and GCC_except_table0 are the part of function foo. Those are exception tables generated by llvm. And I left them when I write this test by hand with a thought that I'd like to check the contents of exception tables. On the other hand, you are right. Everything below .Lfunc_end0: should not have been added since it is the purpose of this UpdateTestChecks/asm.py. I'll find other way to check exception tables.

LGTM.

This revision is now accepted and ready to land.Aug 10 2022, 4:35 PM

Ah, Ok. I've understood what you are trying to fix now. You are trying to correct regexp for VE in UpdateTestChecks/asm.py. I couldn't correct it, so I just hand-write this builtin_sjlj_landingpad.ll test when I created this.

Thank you!

It's looks like this patch itself IS removing landing pad instructions from builtin_sjlj_landingpad.ll although the explanation of this patch is saying opposite.

I re-ran the update script on that file, and now it correctly detects the function boundary: Everything below .Lfunc_end0: should not have been added as a CHECK line but because the regex was wrong before it matched everything up to the next function.

I was saying that removed lines including .LJIT0_0 and GCC_except_table0 are the part of function foo. Those are exception tables generated by llvm. And I left them when I write this test by hand with a thought that I'd like to check the contents of exception tables. On the other hand, you are right. Everything below .Lfunc_end0: should not have been added since it is the purpose of this UpdateTestChecks/asm.py. I'll find other way to check exception tables.

LGTM.

Ah if you actually want to check those tables, you can add manual checks with UTC_ARGS: --disable. Will update this commit

kaz7 added a comment.Aug 22 2022, 1:46 AM

Thank you, but I would like to consider what option is the best for this case. So, please merge this patch as is. Thank you again for fixing the problem in regexp.

This revision was landed with ongoing or failed builds.Aug 24 2022, 7:18 AM
This revision was automatically updated to reflect the committed changes.