This is an archive of the discontinued LLVM Phabricator instance.

Differential D59008

[AMDGPU] Switch default dwarf version to 5
ClosedPublic

Authored by scott.linder on Mar 5 2019, 5:11 PM.

Details

Reviewers
kzhuravl
t-tye
Commits
rGa377be6eb655: [AMDGPU] Switch default DWARF version to 5
rL357285: [AMDGPU] Switch default DWARF version to 5
rC357285: [AMDGPU] Switch default DWARF version to 5
Summary

Reverts r337612. The issues that cropped up with the last attempt appear to have gone away.

Diff Detail

Event Timeline

scott.linder created this revision.Mar 5 2019, 5:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2019, 5:11 PM
Herald added subscribers: cfe-commits, tpr, dstuttard and 4 others. · View Herald Transcript
dblaikie added a subscriber: dblaikie.Mar 11 2019, 2:03 PM
scott.linder added a comment.Mar 25 2019, 10:44 AM

Ping

t-tye accepted this revision.Mar 25 2019, 11:08 AM

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

This revision is now accepted and ready to land.Mar 25 2019, 11:08 AM
dblaikie added a comment.Mar 25 2019, 11:57 AM
In D59008#1441903, @t-tye wrote:

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

State of them in what sense? Compression is pretty orthogonal to any DWARF version - it's more about the container (ELF, etc) you use. Split DWARF is non-standardly supported in pre-v5, and I think it's functioning in the standards conformant v5 mode too.

t-tye added a comment.Mar 25 2019, 1:59 PM
In D59008#1442014, @dblaikie wrote:
In D59008#1441903, @t-tye wrote:

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

State of them in what sense? Compression is pretty orthogonal to any DWARF version - it's more about the container (ELF, etc) you use. Split DWARF is non-standardly supported in pre-v5, and I think it's functioning in the standards conformant v5 mode too.

I had heard mention that at least split DWARF may not be fully supported yet for DWARF 5 in LLVM. But maybe that is old news:-)

kzhuravl accepted this revision.Mar 25 2019, 2:04 PM

LGTM

dblaikie added a comment.Mar 25 2019, 5:02 PM
In D59008#1442256, @t-tye wrote:
In D59008#1442014, @dblaikie wrote:
In D59008#1441903, @t-tye wrote:

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

State of them in what sense? Compression is pretty orthogonal to any DWARF version - it's more about the container (ELF, etc) you use. Split DWARF is non-standardly supported in pre-v5, and I think it's functioning in the standards conformant v5 mode too.

I had heard mention that at least split DWARF may not be fully supported yet for DWARF 5 in LLVM. But maybe that is old news:-)

Might be - nothing I know of right now. (type units aren't fully supported in DWARFv5 - but that's the only big thing on my list)

probinson added a subscriber: probinson.Mar 26 2019, 7:19 AM
In D59008#1442515, @dblaikie wrote:
In D59008#1442256, @t-tye wrote:
In D59008#1442014, @dblaikie wrote:
In D59008#1441903, @t-tye wrote:

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

State of them in what sense? Compression is pretty orthogonal to any DWARF version - it's more about the container (ELF, etc) you use. Split DWARF is non-standardly supported in pre-v5, and I think it's functioning in the standards conformant v5 mode too.

I had heard mention that at least split DWARF may not be fully supported yet for DWARF 5 in LLVM. But maybe that is old news:-)

Might be - nothing I know of right now. (type units aren't fully supported in DWARFv5 - but that's the only big thing on my list)

Anything having to do with section format and placement ought to be correct at this point. Certainly I see v5 type units coming out in the .debug_info section per spec, and the .dwo files look right. If there's something missing please file a bug and CC me.

There was certainly a release where split-dwarf and type units didn't work yet, but that's all done now.

dblaikie added a comment.Mar 26 2019, 8:16 AM
In D59008#1442962, @probinson wrote:
In D59008#1442515, @dblaikie wrote:
In D59008#1442256, @t-tye wrote:
In D59008#1442014, @dblaikie wrote:
In D59008#1441903, @t-tye wrote:

LGTM

Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)?

State of them in what sense? Compression is pretty orthogonal to any DWARF version - it's more about the container (ELF, etc) you use. Split DWARF is non-standardly supported in pre-v5, and I think it's functioning in the standards conformant v5 mode too.

I had heard mention that at least split DWARF may not be fully supported yet for DWARF 5 in LLVM. But maybe that is old news:-)

Might be - nothing I know of right now. (type units aren't fully supported in DWARFv5 - but that's the only big thing on my list)

Anything having to do with section format and placement ought to be correct at this point. Certainly I see v5 type units coming out in the .debug_info section per spec, and the .dwo files look right. If there's something missing please file a bug and CC me.

There was certainly a release where split-dwarf and type units didn't work yet, but that's all done now.

Ah, thanks for the correction - I thought that was the case, and then I thought I'd tested it a couple of weeks ago and seen it wasn't (when I'd expected it was)... must've messed up my test, maybe used an old compiler build I had lying around.

Good stuff!

scott.linder updated this revision to Diff 192688.Mar 28 2019, 11:19 AM

Add a test to confirm split-dwarf is supported for the amdhsa OS in the driver.

Closed by commit rC357285: [AMDGPU] Switch default DWARF version to 5 (authored by scott.linder). · Explain WhyMar 29 2019, 10:45 AM
This revision was automatically updated to reflect the committed changes.
scott.linder mentioned this in D89484: [AMDGPU][HIP] Switch default DWARF version to 5.Oct 15 2020, 10:09 AM
scott.linder mentioned this in rGc4d10e7e9bb4: [AMDGPU][HIP] Switch default DWARF version to 5.Oct 16 2020, 10:54 AM

Revision Contents

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

Diff 189426

lib/Driver/ToolChains/AMDGPU.h

Show First 20 Lines • Show All 49 Lines • ▼ Show 20 Lines const StringRef getOptionDefault(options::ID OptID) const {
auto opt = OptionsDefault.find(OptID); auto opt = OptionsDefault.find(OptID);
assert(opt != OptionsDefault.end() && "No Default for Option"); assert(opt != OptionsDefault.end() && "No Default for Option");
return opt->second; return opt->second;
} }
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 5; }
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, void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args, llvm::opt::ArgStringList &CC1Args,
Action::OffloadKind DeviceOffloadKind) const override; 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

test/Driver/amdgpu-toolchain.c

// RUN: %clang -### -target amdgcn--amdhsa -x assembler -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=AS_LINK %s // RUN: %clang -### -target amdgcn--amdhsa -x assembler -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=AS_LINK %s
// AS_LINK: clang{{.*}} "-cc1as" // AS_LINK: clang{{.*}} "-cc1as"
// AS_LINK: ld.lld{{.*}} "-shared" // AS_LINK: ld.lld{{.*}} "-shared"
// RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
// DWARF_VER: "-dwarf-version=2" // DWARF_VER: "-dwarf-version=5"