Skip to content

[clang][flang][Triple][llvm] Add isOffload function to LangOpts and isGPU function to Triple #126956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sarnex merged 5 commits into from
Mar 28, 2025
Merged

[clang][flang][Triple][llvm] Add isOffload function to LangOpts and isGPU function to Triple #126956

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
044158c
[clang] Add isOffloadingTarget function to LangOpts
sarnex Mar 4, 2025
417a6b0
add two flags
sarnex Mar 21, 2025
7e9f6d1
Merge remote-tracking branch 'upstream/main'
sarnex Mar 21, 2025
3167b3f
remove newline
sarnex Mar 21, 2025
3224283
flang fixes
sarnex Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/LangOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,8 @@ class LangOptions : public LangOptionsBase {
bool AtomicFineGrainedMemory = false;
bool AtomicIgnoreDenormalMode = false;

bool IsOffloadingTarget = false;

LangOptions();

/// Set language defaults for the given input language and
Expand Down Expand Up @@ -838,6 +840,8 @@ class LangOptions : public LangOptionsBase {
return FPExceptionModeKind::FPE_Ignore;
return EM;
}

bool isOffloadingTarget() const { return IsOffloadingTarget; }
};

/// Floating point control options
Expand Down
7 changes: 3 additions & 4 deletions clang/lib/CodeGen/CGOpenMPRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2613,9 +2613,8 @@ void CGOpenMPRuntime::emitDistributeStaticInit(
emitUpdateLocation(CGF, Loc, OMP_IDENT_WORK_DISTRIBUTE);
llvm::Value *ThreadId = getThreadID(CGF, Loc);
llvm::FunctionCallee StaticInitFunction;
bool isGPUDistribute =
CGM.getLangOpts().OpenMPIsTargetDevice &&
(CGM.getTriple().isAMDGCN() || CGM.getTriple().isNVPTX());
bool isGPUDistribute = CGM.getLangOpts().OpenMPIsTargetDevice &&
Copy link
Member Author

@sarnex sarnex Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here (and a few other places) we were checking for AMDGCN, but the new code checks for AMDGPU.

im not sure if theyre basically interchangeable in offloading code, or if i need to be careful, or if the existing code is wrong and we should be using one or the other everywhere

Copy link
Contributor

@shiltian shiltian Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arsenm

Copy link
Contributor

@arsenm arsenm Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tThey are functionally equivalent given that r600 is essentially dead and doesn't support any offload languages. But probably should stick to amdgpu

Copy link
Contributor

@jhuber6 jhuber6 Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we should do a sed s/isAMDGCN()/isAMDGPU()/g at some point?

Copy link
Contributor

@arsenm arsenm Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably

Copy link
Member Author

@sarnex sarnex Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, so new code seems fine then

CGM.getLangOpts().isOffloadingTarget();
StaticInitFunction = OMPBuilder.createForStaticInitFunction(
Values.IVSize, Values.IVSigned, isGPUDistribute);

Expand Down Expand Up @@ -2645,7 +2644,7 @@ void CGOpenMPRuntime::emitForStaticFinish(CodeGenFunction &CGF,
auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
if (isOpenMPDistributeDirective(DKind) &&
CGM.getLangOpts().OpenMPIsTargetDevice &&
(CGM.getTriple().isAMDGCN() || CGM.getTriple().isNVPTX()))
CGM.getLangOpts().isOffloadingTarget())
CGF.EmitRuntimeCall(
OMPBuilder.getOrCreateRuntimeFunction(
CGM.getModule(), OMPRTL___kmpc_distribute_static_fini),
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ static void setVisibilityFromDLLStorageClass(const clang::LangOptions &LO,
static bool isStackProtectorOn(const LangOptions &LangOpts,
const llvm::Triple &Triple,
clang::LangOptions::StackProtectorMode Mode) {
if (Triple.isAMDGPU() || Triple.isNVPTX())
if (LangOpts.isOffloadingTarget())
return false;
return LangOpts.getStackProtector() == Mode;
}
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/CodeGen/CodeGenModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,7 @@ class CodeGenModule : public CodeGenTypeCache {
bool shouldEmitRTTI(bool ForEH = false) {
return (ForEH || getLangOpts().RTTI) && !getLangOpts().CUDAIsDevice &&
!(getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice &&
(getTriple().isNVPTX() || getTriple().isAMDGPU() ||
getTriple().isSPIRV()));
getLangOpts().isOffloadingTarget());
}

/// Get the address of the RTTI descriptor for the given type.
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4357,6 +4357,10 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
Opts.OpenACCMacroOverride = A->getValue();
}

Opts.IsOffloadingTarget =
(Opts.OpenMPIsTargetDevice || Opts.SYCLIsDevice || Opts.CUDAIsDevice) &&
Copy link
Contributor

@jhuber6 jhuber6 Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the concern is that this isn't strictly a language option since it depends on the target. This is more like "isGPUOffloadingTarget." We could possibly split these into two. One goes in the TargetInfo and one goes in the LangOpts?

Copy link
Member Author

@sarnex sarnex Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what would the two flags be? IsOffloading and IsGPU?

Copy link
Contributor

@jhuber6 jhuber6 Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably, yeah.

Copy link
Member Author

@sarnex sarnex Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, back into the depths.

(T.isNVPTX() || T.isAMDGCN() || T.isSPIROrSPIRV());

// FIXME: Eliminate this dependency.
unsigned Opt = getOptimizationLevel(Args, IK, Diags),
OptSize = getOptimizationLevelSize(Args);
Expand Down
16 changes: 4 additions & 12 deletions clang/lib/Sema/SemaType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,8 +1122,7 @@ static QualType ConvertDeclSpecToType(TypeProcessingState &state) {
}
case DeclSpec::TST_int128:
if (!S.Context.getTargetInfo().hasInt128Type() &&
!(S.getLangOpts().SYCLIsDevice || S.getLangOpts().CUDAIsDevice ||
(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsTargetDevice)))
!(S.getLangOpts().isOffloadingTarget()))
S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
<< "__int128";
if (DS.getTypeSpecSign() == TypeSpecifierSign::Unsigned)
Expand Down Expand Up @@ -1168,8 +1167,7 @@ static QualType ConvertDeclSpecToType(TypeProcessingState &state) {
break;
case DeclSpec::TST_float128:
if (!S.Context.getTargetInfo().hasFloat128Type() &&
!S.getLangOpts().SYCLIsDevice && !S.getLangOpts().CUDAIsDevice &&
!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsTargetDevice))
!S.getLangOpts().isOffloadingTarget())
S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
<< "__float128";
Result = Context.Float128Ty;
Expand Down Expand Up @@ -8362,12 +8360,7 @@ static bool verifyValidIntegerConstantExpr(Sema &S, const ParsedAttr &Attr,
/// match one of the standard Neon vector types.
static void HandleNeonVectorTypeAttr(QualType &CurType, const ParsedAttr &Attr,
Sema &S, VectorKind VecKind) {
bool IsTargetCUDAAndHostARM = false;
if (S.getLangOpts().CUDAIsDevice) {
const TargetInfo *AuxTI = S.getASTContext().getAuxTargetInfo();
IsTargetCUDAAndHostARM =
AuxTI && (AuxTI->getTriple().isAArch64() || AuxTI->getTriple().isARM());
}
bool IsTargetOffloading = S.getLangOpts().isOffloadingTarget();

// Target must have NEON (or MVE, whose vectors are similar enough
// not to need a separate attribute)
Expand Down Expand Up @@ -8401,8 +8394,7 @@ static void HandleNeonVectorTypeAttr(QualType &CurType, const ParsedAttr &Attr,
return;

// Only certain element types are supported for Neon vectors.
if (!isPermittedNeonBaseType(CurType, VecKind, S) &&
!IsTargetCUDAAndHostARM) {
if (!isPermittedNeonBaseType(CurType, VecKind, S) && !IsTargetOffloading) {
S.Diag(Attr.getLoc(), diag::err_attribute_invalid_vector_type) << CurType;
Attr.setInvalid();
return;
Expand Down