This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Refactor and extend elf-header-flags-mach tests
ClosedPublic

Authored by scott.linder on Oct 27 2020, 9:29 AM.

Details

Summary
  • Factor out common elements of the input YAML document and use sed to macro replace the run line specific elements.
  • Add checks for the common elements which depend on the ELF class.
  • Use non-numeric suffix for temporary files to avoid merge conflicts.
  • Sort tests by GFX# ascending.
  • Group ELF and YAML tests by GFX#.

Diff Detail

Event Timeline

scott.linder created this revision.Oct 27 2020, 9:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2020, 9:29 AM
scott.linder requested review of this revision.Oct 27 2020, 9:29 AM
scott.linder edited the summary of this revision. (Show Details)Oct 27 2020, 9:29 AM
arsenm added inline comments.Oct 27 2020, 10:10 AM
llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml
1–3

Is using sed actually portable?

scott.linder added inline comments.Oct 27 2020, 10:23 AM
llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml
1–3

I wasn't certain, but it seems to be a de-facto requirement to run check-all:

$ grep -r 'RUN:.* sed' llvm/test | wc -l
152

I made sure not to use any flags not already used in existing tests (i.e. I only use -e)

scott.linder added inline comments.Oct 27 2020, 1:10 PM
llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml
331

Replace this with unique suffix (the GFX# is unique)

339–345

Duplicated run lines

427

Rather than have these as separate blocks, pair them by GFX#, i.e.:

ELF-R600-R600: ...
YAML-R600-R600: ...

ELF-R600-R630: ...
YAML-R600-R630: ...
t-tye added inline comments.Oct 27 2020, 8:21 PM
llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml
391–402

Put in gfx number order.

scott.linder edited the summary of this revision. (Show Details)
  • Remove duplicated RUN lines
  • Group by GFX#
  • Order by GFX#
scott.linder marked an inline comment as done.Oct 28 2020, 11:14 AM
t-tye accepted this revision.Oct 29 2020, 2:24 PM

LGTM

This revision is now accepted and ready to land.Oct 29 2020, 2:24 PM

Remove extra comment character

This revision was landed with ongoing or failed builds.Oct 30 2020, 11:57 AM
This revision was automatically updated to reflect the committed changes.