This is an archive of the discontinued LLVM Phabricator instance.

[MS][ARM64]: Hoist __ImageBase handling into TargetLoweringObjectFileCOFF::lowerRelativeReference, so that all COFF targets get this.
ClosedPublic

Authored by arm-chrjan01 on Jun 5 2018, 7:49 AM.

Details

Summary

Hoist __ImageBase handling into TargetLoweringObjectFileCOFF::lowerRelativeReference, so that all COFF targets get this. Do the same for getSectionForConstant.

Diff Detail

Event Timeline

arm-chrjan01 created this revision.Jun 5 2018, 7:49 AM
rnk added inline comments.Jun 5 2018, 10:22 AM
lib/Target/AArch64/AArch64TargetObjectFile.cpp
82 ↗(On Diff #149983)

Rather than duplicate this, I think we should hoist __ImageBase handling into TargetLoweringObjectFileCOFF::lowerRelativeReference, so that all COFF targets get this.

Also, do you think you'll need to do the same for getSectionForConstant?

arm-chrjan01 retitled this revision from [MS][ARM64]: Add AArch64_WindowsTargetObjectFile and implement AArch64_WindowsTargetObjectFile::lowerRelativeReference to [MS][ARM64]: Hoist __ImageBase handling into TargetLoweringObjectFileCOFF::lowerRelativeReference, so that all COFF targets get this..
arm-chrjan01 edited the summary of this revision. (Show Details)

Refactored as per Reid's comment.

rnk added a comment.Jun 6 2018, 11:54 AM

Needs a test. I think you can copy llvm/test/MC/COFF/ir-to-imgrel.ll to llvm/test/MC/AArch64/ and modify it accordingly. The test is kind of in the wrong location, though. It should really be in llvm/test/CodeGen/(X86|AArch64), since it's an llc test.

include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
167–168

This doesn't look like something clang-format would produce, consider running git clang-format to reformat the lines touched in the patch.

Added test case. Fixed formatting in TargetLoweringObjectFileImpl.h.

rnk accepted this revision.Jun 11 2018, 1:17 PM

lgtm

This revision is now accepted and ready to land.Jun 11 2018, 1:17 PM

Thank you for reviewing Reid. Please could you commit the change for me as I don't have commit rights.

This revision was automatically updated to reflect the committed changes.