Page MenuHomePhabricator

[HIP] Fix --hip-version flag with 0 as component
ClosedPublic

Authored by ashi1 on Thu, Jun 10, 2:28 PM.

Details

Summary

Allow the usage of minor version 0, for hip versions
such as 4.0. Change the default values when performing
version checks.

Diff Detail

Repository
rC Clang

Event Timeline

ashi1 created this revision.Thu, Jun 10, 2:28 PM
ashi1 requested review of this revision.Thu, Jun 10, 2:28 PM
ashi1 added inline comments.
clang/lib/Driver/ToolChains/AMDGPU.cpp
331

@yaxunl do you think we should allow Major version 0?

yaxunl added inline comments.Thu, Jun 10, 8:02 PM
clang/lib/Driver/ToolChains/AMDGPU.cpp
147–148

I don't think we need to change this part.

319–320

is there any reason to use this number? can we just use ~0U ?

331

I think we should allow major==0 since 0.1 is a valid version number.

also we should allow --hip-version=4 and treat it as 4.0.0, i.e. if major is set but minor is missing, then minor is 0. and we need a test for that.

ashi1 updated this revision to Diff 351449.Fri, Jun 11, 7:52 AM
ashi1 marked 3 inline comments as done.

Fixed to yaxunl's comments, using ~0U , and allowing major only component.

yaxunl accepted this revision.Fri, Jun 11, 9:18 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Fri, Jun 11, 9:18 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jun 11, 9:25 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript