Page MenuHomePhabricator

[clang][nfc] Split getOrCheckAMDGPUCodeObjectVersion
ClosedPublic

Authored by JonChesterfield on Thu, Apr 22, 9:00 AM.

Details

Summary

[clang][nfc] Split getOrCheckAMDGPUCodeObjectVersion

Separates detection of deprecated or invalid code object version from
returning the version. Written to avoid any behaviour change.

Precursor to a revision of D98746.

Diff Detail

Event Timeline

JonChesterfield requested review of this revision.Thu, Apr 22, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 22, 9:00 AM
clang/lib/Driver/ToolChains/CommonArgs.cpp
1610

This would probably be more useful if it diagnosed an invalid argument even when overridden by a legacy option. This patch preserves the existing behaviour.

1623

getAsInteger writes to CodeObjVer.

yaxunl added inline comments.Thu, Apr 22, 1:23 PM
clang/lib/Driver/ToolChains/CommonArgs.cpp
1605–1611

can we just call getAMDGPUCodeObjectVersion here instead of duplicating the logic?

1623

can we return Optional<unsigned> and return None if there is a remnant? Then we can call this function in checkAMDGPUCodeObjectVersion

clang/lib/Driver/ToolChains/CommonArgs.cpp
1623

We could. It would spawn handling of the Optional<> at all the call sites though. I'll see if I can pull the search logic into a local helper.

clang/lib/Driver/ToolChains/CommonArgs.cpp
1623

Implemented in D101117.

  • factor out last arg wins
clang/lib/Driver/ToolChains/CommonArgs.cpp
1576–1586

The logic to pick which of multiple arguments wins seems an important thing to keep in one place. I think this is an improvement on the first draft and on D101117

yaxunl accepted this revision.Thu, Apr 22, 4:11 PM

LGTM. Thanks.

This revision is now accepted and ready to land.Thu, Apr 22, 4:11 PM
This revision was landed with ongoing or failed builds.Thu, Apr 22, 4:25 PM
This revision was automatically updated to reflect the committed changes.