This is an archive of the discontinued LLVM Phabricator instance.

Differential D53059

[NFC] Fix the regular expression for BE PPC in update_llc_test_checks.py
ClosedPublic

Authored by nemanjai on Oct 9 2018, 9:17 PM.

Details

Reviewers
MaskRay
RKSimon
gbedwell
Commits
rGfce57f586d0e: [NFC] Fix the regular expression for BE PPC in update_llc_test_checks.py
rL345363: [NFC] Fix the regular expression for BE PPC in update_llc_test_checks.py
Summary

Currently, the regular expression that matches the lines of assembly for PPC LE (ELFv2) does not work for the assembly for BE (ELFv1). This patch fixes it. Keep in mind that I really don't know Python, so this may not be the way we want to fix it, but it gets the job done on the simple input I used it on.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Oct 9 2018, 9:17 PM
MaskRay added inline comments.Oct 9 2018, 9:57 PM
utils/UpdateTestChecks/asm.py
53

Can this be merged with ASM_FUNCTION_PPC_RE? If not, is it possible to extract the common part to a variable ASM_FUNCTION_PPC_COMMON (you make give it a better name)?

245

The leading tab should be replaced with spaces.

nemanjai added a comment.Oct 9 2018, 10:57 PM

Thanks for your comments @MaskRay. Let me know if you think that RE would work.

utils/UpdateTestChecks/asm.py
53

To be honest with you, this regular expression syntax is mostly gibberish to me. From what I was able to decipher, the first line looks for function_name: # @function_name which appears in PPC asm syntax. Then for LE, the following line is .Lfunc_beginN: whereas for BE, there are a few more lines with .p2align, .quad, .text.
Perhaps it is possible to match both with something like .*?
i.e. match anything non-greedy until you encounter what's in the next RE. And the next thing will be the .Lfunc_beginN:

245

OK.

MaskRay added inline comments.Oct 10 2018, 12:32 PM
utils/UpdateTestChecks/asm.py
53

Yeah if .p2align .quad .text are not that important, maybe a catch-all .* is better..

RKSimon added a reviewer: gbedwell.Oct 22 2018, 5:24 AM
nemanjai updated this revision to Diff 170410.Oct 22 2018, 7:41 AM

Updated the regular expression to just eat any text that is not relevant between the function directive and its text. This seems to work for both ELFv1 and ELFv2 asm output.

MaskRay accepted this revision.Oct 22 2018, 10:17 AM
This revision is now accepted and ready to land.Oct 22 2018, 10:17 AM
Closed by commit rL345363: [NFC] Fix the regular expression for BE PPC in update_llc_test_checks.py (authored by nemanjai). · Explain WhyOct 25 2018, 8:33 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
utils/
UpdateTestChecks/
17 lines

Diff 168937

utils/UpdateTestChecks/asm.py

Show First 20 Lines • Show All 44 Lines • ▼ Show 20 LinesASM_FUNCTION_MIPS_RE = re.compile(
r'^_?(?P<func>[^:]+):[ \t]*#+[ \t]*@(?P=func)\n[^:]*?' # f: (name of func) r'^_?(?P<func>[^:]+):[ \t]*#+[ \t]*@(?P=func)\n[^:]*?' # f: (name of func)
r'(?:^[ \t]+\.(frame|f?mask|set).*?\n)+' # Mips+LLVM standard asm prologue r'(?:^[ \t]+\.(frame|f?mask|set).*?\n)+' # Mips+LLVM standard asm prologue
r'(?P<body>.*?)\n' # (body of the function) r'(?P<body>.*?)\n' # (body of the function)
r'(?:^[ \t]+\.(set|end).*?\n)+' # Mips+LLVM standard asm epilogue r'(?:^[ \t]+\.(set|end).*?\n)+' # Mips+LLVM standard asm epilogue
r'(\$|\.L)func_end[0-9]+:\n', # $func_end0: (mips32 - O32) or r'(\$|\.L)func_end[0-9]+:\n', # $func_end0: (mips32 - O32) or
# .Lfunc_end0: (mips64 - NewABI) # .Lfunc_end0: (mips64 - NewABI)
flags=(re.M | re.S)) flags=(re.M | re.S))
ASM_FUNCTION_PPC_BE_RE = re.compile(
MaskRayUnsubmitted
Not Done
ReplyInline Actions

Can this be merged with ASM_FUNCTION_PPC_RE? If not, is it possible to extract the common part to a variable ASM_FUNCTION_PPC_COMMON (you make give it a better name)?

MaskRay: Can this be merged with `ASM_FUNCTION_PPC_RE`? If not, is it possible to extract the common…
nemanjaiAuthorUnsubmitted
Not Done
ReplyInline Actions

To be honest with you, this regular expression syntax is mostly gibberish to me. From what I was able to decipher, the first line looks for function_name: # @function_name which appears in PPC asm syntax. Then for LE, the following line is .Lfunc_beginN: whereas for BE, there are a few more lines with .p2align, .quad, .text.
Perhaps it is possible to match both with something like .*?
i.e. match anything non-greedy until you encounter what's in the next RE. And the next thing will be the .Lfunc_beginN:

nemanjai: To be honest with you, this regular expression syntax is mostly gibberish to me. From what I…
MaskRayUnsubmitted
Not Done
ReplyInline Actions

Yeah if .p2align .quad .text are not that important, maybe a catch-all .* is better..

MaskRay: Yeah if .p2align .quad .text are not that important, maybe a catch-all `.*` is better..
r'^_?(?P<func>[^:]+):[ \t]*#+[ \t]*@(?P=func)\n'
r'.*?\n[ \t]+.text\n'
r'\.Lfunc_begin[0-9]+:\n'
r'(?:[ \t]+.cfi_startproc\n)?'
r'(?:\.Lfunc_[gl]ep[0-9]+:\n(?:[ \t]+.*?\n)*)*'
r'(?P<body>.*?)\n'
# This list is incomplete
r'(?:^[ \t]*(?:\.long[ \t]+[^\n]+|\.quad[ \t]+[^\n]+)\n)*'
r'.Lfunc_end[0-9]+:\n',
flags=(re.M | re.S))
ASM_FUNCTION_PPC_RE = re.compile( ASM_FUNCTION_PPC_RE = re.compile(
r'^_?(?P<func>[^:]+):[ \t]*#+[ \t]*@(?P=func)\n' r'^_?(?P<func>[^:]+):[ \t]*#+[ \t]*@(?P=func)\n'
r'\.Lfunc_begin[0-9]+:\n' r'\.Lfunc_begin[0-9]+:\n'
r'(?:[ \t]+.cfi_startproc\n)?' r'(?:[ \t]+.cfi_startproc\n)?'
r'(?:\.Lfunc_[gl]ep[0-9]+:\n(?:[ \t]+.*?\n)*)*' r'(?:\.Lfunc_[gl]ep[0-9]+:\n(?:[ \t]+.*?\n)*)*'
r'(?P<body>.*?)\n' r'(?P<body>.*?)\n'
# This list is incomplete # This list is incomplete
r'(?:^[ \t]*(?:\.long[ \t]+[^\n]+|\.quad[ \t]+[^\n]+)\n)*' r'(?:^[ \t]*(?:\.long[ \t]+[^\n]+|\.quad[ \t]+[^\n]+)\n)*'
▲ Show 20 Lines • Show All 164 Lines • ▼ Show 20 Lines target_handlers = {
'thumbv8m.main': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'thumbv8m.main': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armv6': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armv6': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armv7': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armv7': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armv7-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armv7-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armeb-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armeb-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armv7eb-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armv7eb-eabi': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'armv7eb': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE), 'armv7eb': (scrub_asm_arm_eabi, ASM_FUNCTION_ARM_RE),
'mips': (scrub_asm_mips, ASM_FUNCTION_MIPS_RE), 'mips': (scrub_asm_mips, ASM_FUNCTION_MIPS_RE),
'powerpc64': (scrub_asm_powerpc64, ASM_FUNCTION_PPC_RE), # Including the dashes in the triple so PPC BE does not subsume LE.
MaskRayUnsubmitted
Not Done
ReplyInline Actions

The leading tab should be replaced with spaces.

MaskRay: The leading tab should be replaced with spaces.
nemanjaiAuthorUnsubmitted
Not Done
ReplyInline Actions

OK.

nemanjai: OK.
'powerpc64le': (scrub_asm_powerpc64, ASM_FUNCTION_PPC_RE), 'powerpc64-': (scrub_asm_powerpc64, ASM_FUNCTION_PPC_BE_RE),
'powerpc64le-': (scrub_asm_powerpc64, ASM_FUNCTION_PPC_RE),
'riscv32': (scrub_asm_riscv, ASM_FUNCTION_RISCV_RE), 'riscv32': (scrub_asm_riscv, ASM_FUNCTION_RISCV_RE),
'riscv64': (scrub_asm_riscv, ASM_FUNCTION_RISCV_RE), 'riscv64': (scrub_asm_riscv, ASM_FUNCTION_RISCV_RE),
'sparc': (scrub_asm_sparc, ASM_FUNCTION_SPARC_RE), 'sparc': (scrub_asm_sparc, ASM_FUNCTION_SPARC_RE),
'sparcv9': (scrub_asm_sparc, ASM_FUNCTION_SPARC_RE), 'sparcv9': (scrub_asm_sparc, ASM_FUNCTION_SPARC_RE),
's390x': (scrub_asm_systemz, ASM_FUNCTION_SYSTEMZ_RE), 's390x': (scrub_asm_systemz, ASM_FUNCTION_SYSTEMZ_RE),
} }
handlers = None handlers = None
for prefix, s in target_handlers.items(): for prefix, s in target_handlers.items():
Show All 17 Lines