This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Default to hidden visibility
ClosedPublic

Authored by arsenm on Aug 24 2018, 2:23 AM.

Download Raw Diff

Details

Reviewers

t-tye
kzhuravl
yaxunl

Summary

Object linking isn't supported, so it's not useful
to emit default visibility. Default visibility requires
relocations we don't yet support for functions compiled
in another translation unit.

WebAssembly already does this, although they insert these
arguments in a different place for some reason.

Diff Detail

Event Timeline

arsenm created this revision.Aug 24 2018, 2:23 AM

Herald added subscribers: sunfish, aheejin, tpr and 5 others. · View Herald TranscriptAug 24 2018, 2:23 AM

nhaehnle removed a subscriber: nhaehnle.Aug 24 2018, 4:20 AM

arsenm mentioned this in D51434: [HIP] Add -fvisibility hidden option to clang.Aug 29 2018, 11:22 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Aug 29 2018, 1:55 PM

r341033

Revision Contents

Path

Size

lib/

Driver/

ToolChains/

AMDGPU.h

4 lines

AMDGPU.cpp

13 lines

test/

Driver/

amdgpu-visibility.cl

7 lines

Diff 162350

lib/Driver/ToolChains/AMDGPU.h

	Show First 20 Lines • Show All 55 Lines • ▼ Show 20 Lines
	public:			public:
	AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,			AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
	const llvm::opt::ArgList &Args);			const llvm::opt::ArgList &Args);
	unsigned GetDefaultDwarfVersion() const override { return 2; }			unsigned GetDefaultDwarfVersion() const override { return 2; }
	bool IsIntegratedAssemblerDefault() const override { return true; }			bool IsIntegratedAssemblerDefault() const override { return true; }
	llvm::opt::DerivedArgList *			llvm::opt::DerivedArgList *
	TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,			TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
	Action::OffloadKind DeviceOffloadKind) const override;			Action::OffloadKind DeviceOffloadKind) const override;

				void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
				llvm::opt::ArgStringList &CC1Args,
				Action::OffloadKind DeviceOffloadKind) const override;
	};			};

	} // end namespace toolchains			} // end namespace toolchains
	} // end namespace driver			} // end namespace driver
	} // end namespace clang			} // end namespace clang

	#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_AMDGPU_H			#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_AMDGPU_H

lib/Driver/ToolChains/AMDGPU.cpp

Show First 20 Lines • Show All 92 Lines • ▼ Show 20 Lines	if (Args.hasArg(options::OPT_c) && Args.hasArg(options::OPT_emit_llvm)) {
if (!Args.hasArg(options::OPT_O, options::OPT_O0, options::OPT_O4,		if (!Args.hasArg(options::OPT_O, options::OPT_O0, options::OPT_O4,
options::OPT_Ofast))		options::OPT_Ofast))
DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O),		DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O),
getOptionDefault(options::OPT_O));		getOptionDefault(options::OPT_O));
}		}

return DAL;		return DAL;
}		}

		void AMDGPUToolChain::addClangTargetOptions(
		const llvm::opt::ArgList &DriverArgs,
		llvm::opt::ArgStringList &CC1Args,
		Action::OffloadKind DeviceOffloadingKind) const {
		// Default to "hidden" visibility, as object level linking will not be
		// supported for the forseeable future.
		if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
		options::OPT_fvisibility_ms_compat)) {
		CC1Args.push_back("-fvisibility");
		CC1Args.push_back("hidden");
		}
		}

test/Driver/amdgpu-visibility.cl

This file was added.

				// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm %s 2>&1 \| FileCheck -check-prefix=DEFAULT %s
				// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm -fvisibility=protected %s 2>&1 \| FileCheck -check-prefix=OVERRIDE-PROTECTED %s
				// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm -fvisibility-ms-compat %s 2>&1 \| FileCheck -check-prefix=OVERRIDE-MS %s

				// DEFAULT: "-fvisibility" "hidden"
				// OVERRIDE-PROTECTED: "-fvisibility" "protected"
				// OVERRIDE-MS: "-fvisibility" "hidden" "-ftype-visibility" "default"