Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Make code size metric names independent of platform
AbandonedPublic

Authored by paquette on Mar 16 2023, 2:14 PM.

Details

Summary

On Darwin/mach-o, llvm-size (and size) will output something like

__text

While on other platforms (e.g. GNU) you'll get

.text

codesize.py was basically pushing that platform-specific info along in its size
related output.

So, depending on the platform, we'd get

size.__text
size..text

Tools downstream from the test suite then would have to handle whatever output
the target-specific tools would produce.

Instead of doing that, let's just output something like

size_text

So all downstream consumers can just handle one single format.

This is in tandem with D146257 in LNT.

Diff Detail

Repository
rT test-suite

Event Timeline

paquette created this revision.Mar 16 2023, 2:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2023, 2:14 PM
fhahn accepted this revision.Mar 17 2023, 3:35 AM

LGTM, thanks!

@cmatthews @azhar do you know if there are any tests for this part of the code?

This revision is now accepted and ready to land.Mar 17 2023, 3:35 AM
paquette abandoned this revision.Mar 20 2023, 3:06 PM

Over in D146257 we decided that we don't want to change the test suite, and would prefer the change to be local to LNT. We don't need this change.

@jpaquette, what do you think?

litsupport/modules/codesize.py
42

I was just thinking maybe, as a good measure, we can assert here that only one of size.__text or size..text is present in metrics.

assert not ('size.__text' in metrics and 'size..text' in metrics), "Both 'size.__text' and 'size..text' present in metrics. Only one of them should exist."
paquette added inline comments.Mar 21 2023, 12:27 PM
litsupport/modules/codesize.py
42

Sure, that sounds like a good idea. Lemme spin up another patch.