Page MenuHomePhabricator

[NFC] Add the getSize() interface for MachineConstantPoolValue
Needs ReviewPublic

Authored by steven.zhang on Oct 9 2020, 2:23 AM.

Details

Reviewers
nemanjai
MaskRay
echristo
RKSimon
lebedev.ri
Group Reviewers
Restricted Project
Summary

Current implementation assumes that, each MachineConstantPoolValue takes up sizeof(MachineConstantPoolValue::Ty) bytes. For PowerPC, we want to lump all the constants with the same type as one MachineConstantPoolValue to save the cost that calculate the TOC entry for each const. So, we need to extend the MachineConstantPoolValue that break this assumption.
That is, transform:

addis 3, 2, .LCPI0_0@toc@ha
lfd 0, .LCPI0_0@toc@l(3)
addis 3, 2, .LCPI0_1@toc@ha
lfd 1, .LCPI0_1@toc@l(3)

to:

addis 3, 2, .LC0@toc@ha
ld 3, .LC0@toc@l(3)
lfd 0, 8(3)
lfd 1, 0(3)

And

.LCPI0_0:
        .quad   0x40218d4fdf3b645a              # double 8.7759999999999998
.LCPI0_1:
        .quad   0x49818d4fdf3b645a              # double 1.2525525751187504E+46

->

.LCPI0_0:
        .quad   0x49818d4fdf3b645a              # double 1.2525525751187504E+46
        .quad   0x40218d4fdf3b645a              # double 8.7759999999999998
        .section        ".note.GNU-stack","",@progbits
        .section        .toc,"aw",@progbits
.LC0:
        .tc .LCPI0_0[TC],.LCPI0_0

The more constants accessed, the more benefit we will have.

And this assumption also doesn't make sense to me as we have already provided mechanism for target to customize the constant pool with MachineConstantPoolValue. We shouldn't assume its size.

Diff Detail

Unit TestsFailed

TimeTest
3,920 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.AddFiles
Note: Google Test filter = DirectoryWatcherTest.AddFiles [==========] Running 1 test from 1 test case.
3,740 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.DeleteFile
Note: Google Test filter = DirectoryWatcherTest.DeleteFile [==========] Running 1 test from 1 test case.
3,840 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.ModifyFile
Note: Google Test filter = DirectoryWatcherTest.ModifyFile [==========] Running 1 test from 1 test case.
160 mswindows > lld.ELF/invalid::symtab-sh-info.s
Script: -- : 'RUN: at line 4'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\yaml2obj.exe --docnum=1 C:\ws\w16n2-1\llvm-project\premerge-checks\lld\test\ELF\invalid\symtab-sh-info.s -o C:\ws\w16n2-1\llvm-project\premerge-checks\build\tools\lld\test\ELF\invalid\Output\symtab-sh-info.s.tmp.o

Event Timeline

steven.zhang created this revision.Oct 9 2020, 2:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 9 2020, 2:23 AM
steven.zhang requested review of this revision.Oct 9 2020, 2:23 AM
steven.zhang edited the summary of this revision. (Show Details)
steven.zhang added inline comments.Oct 9 2020, 2:27 AM
llvm/lib/Target/X86/X86MCInstLower.cpp
1784

The assertion here won't help too much as ConstantEntry.getType() is essential C->getType(). Remove this assertion to remove the getType() from MachineConstantPoolEntry.

lebedev.ri resigned from this revision.Oct 9 2020, 2:33 AM

TOC is a step forward from GOT for which one can control the layout. This kind of optimizations should be done (otherwise we can generate @got in the first place).

llvm/include/llvm/CodeGen/MachineConstantPool.h
46

Name - is deprecated in the coding standard. Please drop.

Actually the function name self explains so perhaps the comment can be omitted.

Address comments.

Can you please post the dependent patch to make it clear whether this refactoring is suitable?

llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
513

if const unsigned works, please change as well.

Can you please post the dependent patch to make it clear whether this refactoring is suitable?

Ok, I will post the dependent patches later.

llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
513

Sorry, do you mean const unsigned Size = ... ? I don't think there is any benefit to have the cv-qualifier here but increasing the code size.