This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add code size information on isFPImmLegal
ClosedPublic

Authored by zatrazz on Feb 26 2019, 11:11 AM.

Details

Summary

This allow to avoid materialize constants which would required 2 instructions (mov plus movk for instance) when optimizing for size.

Diff Detail

Event Timeline

zatrazz created this revision.Feb 26 2019, 11:11 AM

TargetLoweringBase::isFPImmLegal() could have a default value of false for ForCodeSize.

include/llvm/CodeGen/TargetLowering.h
782

CamelCase.

lib/Target/AArch64/AArch64ISelLowering.cpp
5395

Ditto.

lib/Target/AArch64/AArch64ISelLowering.h
290

Ditto.

lib/Target/AMDGPU/AMDGPUISelLowering.cpp
643

Ditto.

lib/Target/AMDGPU/AMDGPUISelLowering.h
165

Ditto.

lib/Target/ARM/ARMISelLowering.cpp
14375

Ditto.

lib/Target/ARM/ARMISelLowering.h
482

Ditto.

lib/Target/Hexagon/HexagonISelLowering.cpp
2928

Ditto.

lib/Target/Hexagon/HexagonISelLowering.h
288

Ditto.

lib/Target/Mips/MipsISelLowering.cpp
4150

Ditto.

lib/Target/Mips/MipsISelLowering.h
679

Ditto.

lib/Target/PowerPC/PPCISelLowering.cpp
14326

Ditto.

lib/Target/PowerPC/PPCISelLowering.h
891

Ditto.

lib/Target/SystemZ/SystemZISelLowering.cpp
695

Ditto.

lib/Target/SystemZ/SystemZISelLowering.h
404

Ditto.

lib/Target/X86/X86ISelLowering.cpp
4804

Ditto.

lib/Target/X86/X86ISelLowering.h
1013

Ditto.

zatrazz updated this revision to Diff 189175.Mar 4 2019, 11:36 AM

Updated patch based on previous comments.

zatrazz updated this revision to Diff 189751.Mar 7 2019, 10:34 AM

Fxied CamelCase (sorry missing the comment).

evandro added inline comments.Mar 11 2019, 11:54 AM
lib/Target/AArch64/AArch64ISelLowering.cpp
5407

Oops!

lib/Target/ARM/ARMISelLowering.cpp
4177

Should this be in another patch?

4182

Ditto.

4188

Ditto.

4216

Ditto.

13709

?

lib/Target/SystemZ/SystemZISelLowering.cpp
3742

Ditto.

lib/Target/SystemZ/SystemZISelLowering.h
646

Should this be in another patch?

lib/Target/X86/X86ISelLowering.cpp
6909

Ditto.

7479

Ditto.

7510

Ditto.

28873

Ditto.

32820

Ditto.

41950

Ditto.

zatrazz marked an inline comment as done.Mar 11 2019, 1:41 PM
zatrazz added inline comments.
lib/Target/AArch64/AArch64ISelLowering.cpp
5407

Hi Evandro, I am not following your remarks. At least on phabricator they are pointing on code section that has not been actually changed. Should I submit the patch again?

evandro added inline comments.Mar 11 2019, 1:50 PM
lib/Target/AArch64/AArch64ISelLowering.cpp
5407

I don't know what Phabricator showed me before either. Please, ignore my previous review. I'll go over the patch again.

evandro accepted this revision.Mar 11 2019, 1:57 PM

LGTM, after the minor issues below are addressed.

lib/Target/AMDGPU/AMDGPUISelLowering.h
165

Indentation.

lib/Target/ARM/ARMFastISel.cpp
427

false is already the default value.

lib/Target/ARM/ARMISelLowering.cpp
5887

Ditto.

This revision is now accepted and ready to land.Mar 11 2019, 1:57 PM
zatrazz closed this revision.Mar 18 2019, 11:38 AM