This is an archive of the discontinued LLVM Phabricator instance.

Differential D92628

[HIP] Fix bug in driver about wavefront size
ClosedPublic

Authored by yaxunl on Dec 3 2020, 7:01 PM.

Details

Reviewers
tra
arsenm
Commits
rG0519e1ddb388: [HIP] Fix bug in driver about wavefront size
Summary

The static variable causes it only initialized once and take
the same value for different GPU archs, whereas they
may be different for different GPU archs, e.g. when
there are both gfx900 and gfx1010.

Removing static fixes that.

Diff Detail

Event Timeline

yaxunl created this revision.Dec 3 2020, 7:01 PM
Herald added subscribers: kerbowa, nhaehnle, jvesely. · View Herald TranscriptDec 3 2020, 7:01 PM
yaxunl requested review of this revision.Dec 3 2020, 7:01 PM
Herald added a subscriber: wdng. · View Herald TranscriptDec 3 2020, 7:01 PM
arsenm accepted this revision.Dec 3 2020, 7:26 PM

LGTM although I thought there were existing test files for this

This revision is now accepted and ready to land.Dec 3 2020, 7:26 PM
Closed by commit rG0519e1ddb388: [HIP] Fix bug in driver about wavefront size (authored by yaxunl). · Explain WhyDec 4 2020, 5:37 AM
This revision was automatically updated to reflect the committed changes.
yaxunl added a commit: rG0519e1ddb388: [HIP] Fix bug in driver about wavefront size.
Herald added a project: Restricted Project. · View Herald TranscriptDec 4 2020, 5:37 AM

Revision Contents

PathSize
clang/
lib/
Driver/
ToolChains/
2 lines
test/
Driver/
21 lines

Diff 309520

clang/lib/Driver/ToolChains/AMDGPU.cpp

Show First 20 Lines • Show All 493 Lines • ▼ Show 20 Linesllvm::DenormalMode AMDGPUToolChain::getDefaultDenormalModeForType(
// also implicit treated as zero (DAZ). // also implicit treated as zero (DAZ).
return DAZ ? llvm::DenormalMode::getPreserveSign() : return DAZ ? llvm::DenormalMode::getPreserveSign() :
llvm::DenormalMode::getIEEE(); llvm::DenormalMode::getIEEE();
} }
bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs, bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs,
llvm::AMDGPU::GPUKind Kind) { llvm::AMDGPU::GPUKind Kind) {
const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind); const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
static bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32); bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
return !HasWave32 || DriverArgs.hasFlag( return !HasWave32 || DriverArgs.hasFlag(
options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false); options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
} }
/// ROCM Toolchain /// ROCM Toolchain
ROCMToolChain::ROCMToolChain(const Driver &D, const llvm::Triple &Triple, ROCMToolChain::ROCMToolChain(const Driver &D, const llvm::Triple &Triple,
▲ Show 20 Lines • Show All 132 Lines • Show Last 20 Lines

clang/test/Driver/hip-wavefront-size.hip

  • This file was added.
// REQUIRES: clang-driver,amdgpu-registered-target
// RUN: %clang -### -target x86_64-linux-gnu \
// RUN: --cuda-gpu-arch=gfx900 \
// RUN: --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
// RUN: 2>&1 | FileCheck %s --check-prefixes=WAVE64
// WAVE64: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
// RUN: %clang -### -target x86_64-linux-gnu \
// RUN: --cuda-gpu-arch=gfx1010 \
// RUN: --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
// RUN: 2>&1 | FileCheck %s --check-prefixes=WAVE32
// WAVE32: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"
// RUN: %clang -### -target x86_64-linux-gnu \
// RUN: --cuda-gpu-arch=gfx1010 \
// RUN: --cuda-gpu-arch=gfx900 \
// RUN: --rocm-path=%S/Inputs/rocm --cuda-device-only %s \
// RUN: 2>&1 | FileCheck %s --check-prefixes=BOTH
// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"{{.*}} "-target-cpu" "gfx900"
// BOTH-DAG: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_off.bc"{{.*}} "-target-cpu" "gfx1010"