This is an archive of the discontinued LLVM Phabricator instance.

Renumber testcase metadata nodes after D26769. [NFC]
ClosedPublic

Authored by aprantl on Dec 14 2016, 10:39 AM.

Details

Summary

This patch renumbers the metadata nodes in debug info testcases after https://reviews.llvm.org/D26769. This is a separate patch because it causes so much churn. This was implemented with a python script that pipes the testcases through llvm-as - | llvm-dis - and then goes through the original and new output side-by side to insert all comments at a close-enough location.

Here's the script for reference:

import re, sys, subprocess
fn = sys.argv[1]
stdout = subprocess.check_output('llvm-as %s -o - | llvm-dis -o -' % fn, shell=True)
new = stdout.split('\n')

out = []
comment = re.compile(r'^([^;]*)(;.*)$')
ws = re.compile(r'^ *$')
empty = re.compile(r'^ *\n$')
for line in open(fn):
    if new and new[0].startswith('; ModuleID ='):
        new = new[1:]
    # If there was a comment on this line, carry it over.
    m = comment.match(line)
    if m:
        if ws.match(m.group(1)):
            if m.group(2).startswith('; Function Attrs'):
                continue
            out.append(line)
        else:
            cmnt = m.group(2)
            if cmnt.startswith('; preds =') or cmnt.count(r'#uses='):
                cmnt = ''
            out.append(new[0]+' '+cmnt+'\n')
            new = new[1:]
    else:
        if empty.match(line):
            if out[-1] <> '\n':
                out.append('\n')
            continue
        # Try to sync the original and the new output again by comparing the
        # first couple of characters.
        if new:
            if new[0] == '' and out and out[-1] == '\n':
                new = new[1:]
                continue
            out.append(new[0]+'\n')
            new = new[1:]
        if len(new) > 2 and new[2][:8] == line[:8]:
            out.append(new[0]+'\n')
            new = new[1:]
        if len(new) > 1 and new[1][:8] == line[:8]:
            out.append(new[0]+'\n')
            new = new[1:]
        if len(new) > 0 and new[0][:8] == line[:8]:
            out.append(new[0]+'\n')
            new = new[1:]
        
f = open(fn, 'w')
f.write(''.join(out+new)+'\n')

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl updated this revision to Diff 81417.Dec 14 2016, 10:39 AM
aprantl retitled this revision from to Renumber testcase metadata nodes after D26769. [NFC].
aprantl updated this object.
aprantl added a reviewer: dblaikie.
aprantl added a subscriber: llvm-commits.
dblaikie accepted this revision.Dec 14 2016, 10:59 AM
dblaikie edited edge metadata.

Looks plausible - spot checked a few spots. Optional feedback if you want to improve those, etc.

test/CodeGen/AArch64/arm64-2011-03-17-AsmPrinterCrash.ll
23–44 ↗(On Diff #81417)

Is dropping all this debug info intentional?

& theer are other 'interesting' changes in other files, but rather than discussing all of them:

Perhaps it'd be better to just run this script over the files that are changed by your previous commit? Ah, I guess that's what you're already doing.

*ponders*

test/DebugInfo/ARM/big-endian-bitfield.ll
39 ↗(On Diff #81417)

This one got thrown off by the DIBasicType here - worth considering a fix to the script to improve this case?

test/DebugInfo/COFF/globals-discarded.ll
31 ↗(On Diff #81417)

Does the script insert blank lines? Might not be worth doing so in a block of contiguous IR without any comments, etc?

This revision is now accepted and ready to land.Dec 14 2016, 10:59 AM
aprantl added inline comments.Dec 14 2016, 11:18 AM
test/CodeGen/AArch64/arm64-2011-03-17-AsmPrinterCrash.ll
23–44 ↗(On Diff #81417)

Lots of testcases have unreachable MDNodes in them that are leftover from previous lossy conversions. The script has to drop them.

You're correct, I only ran it on the files containing a DIGlobalVariableExpression and tthat survived the llvm-as | llvm-dis loop. There are a bunch of testcases that don't even do that because of incomplete metadata (missing version etc.). I'll fix up those manually. In a separate commit.

test/DebugInfo/ARM/big-endian-bitfield.ll
39 ↗(On Diff #81417)

I can have the sync heuristic also ignore ![0-9]. Maybe that works.

test/DebugInfo/COFF/globals-discarded.ll
31 ↗(On Diff #81417)

I can modify it to only insert a blank line if the previous line contained a ;.

aprantl updated this revision to Diff 81447.Dec 14 2016, 1:07 PM
aprantl edited edge metadata.

Improved the sync heuristic. Re-uploading for visual inspection.

This revision was automatically updated to reflect the committed changes.