This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Implement target hook function to decide folding (mul (add x, c1), c2)
ClosedPublic

Authored by benshi001 on Aug 28 2021, 8:40 AM.

Diff Detail

Event Timeline

benshi001 created this revision.Aug 28 2021, 8:40 AM
benshi001 requested review of this revision.Aug 28 2021, 8:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2021, 8:40 AM
benshi001 updated this revision to Diff 369317.Aug 29 2021, 6:50 AM
Matt added a subscriber: Matt.Aug 30 2021, 7:42 AM
dmgreen added inline comments.Sep 1 2021, 3:39 PM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12152

Can this use AArch64TTIImpl::getIntImmCost in some way?

benshi001 marked an inline comment as done.Sep 2 2021, 12:47 AM
benshi001 added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12152

I think it is hard. Since the API

TargetTransformInfo
AArch64TargetMachine::getTargetTransformInfo(const Function &F) {
  return TargetTransformInfo(AArch64TTIImpl(this, F));
}

requires a Function object, but the target hook has only two SDValue parameters.

So the cheapest way is implementing a new but short isMOVZImm helper function.

benshi001 marked an inline comment as done.Sep 2 2021, 1:30 AM
benshi001 updated this revision to Diff 370202.Sep 2 2021, 2:22 AM
benshi001 added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12152

I have simplified it to one additional helper function.

dmgreen added inline comments.Sep 3 2021, 6:28 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12152

It's more about not repeating the same (or similar) code. Can this at least use AArch64_IMM::expandMOVImm and check the number of instructions?

benshi001 updated this revision to Diff 370586.Sep 3 2021, 8:08 AM
benshi001 marked an inline comment as done.
benshi001 added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12152

Thanks for your suggestion. Using AArch64_IMM::expandMOVImm makes the check more clear and accurate !!!

benshi001 updated this revision to Diff 370587.Sep 3 2021, 8:10 AM
benshi001 marked an inline comment as done.
dmgreen accepted this revision.Sep 3 2021, 8:49 AM

Thanks. LGTM

This revision is now accepted and ready to land.Sep 3 2021, 8:49 AM
kda added a subscriber: kda.Sep 3 2021, 6:10 PM

Reverting.
Broke buildbot: https://lab.llvm.org/buildbot/#/builders/5/builds/11411

salient log snippet:

Testing:  0.. 10.. 20.. 30.. 40
FAIL: LLVM :: CodeGen/AArch64/urem-seteq-nonzero.ll (34455 of 78731)
******************** TEST 'LLVM :: CodeGen/AArch64/urem-seteq-nonzero.ll' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc -mtriple=aarch64-unknown-linux-gnu < /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/AArch64/urem-seteq-nonzero.ll | /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/AArch64/urem-seteq-nonzero.ll
--
Exit Code: 2
Command Output (stderr):
--
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12208:58: runtime error: signed integer overflow: -2 * -6148914691236517205 cannot be represented in type 'long'
    #0 0x3967870 in llvm::AArch64TargetLowering::isMulAddWithConstProfitable(llvm::SDValue const&, llvm::SDValue const&) const /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12208:58
    #1 0x5cfdcde in (anonymous namespace)::DAGCombiner::isMulAddWithConstProfitable(llvm::SDNode*, llvm::SDValue&, llvm::SDValue&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16879:11
    #2 0x5cb3dc3 in (anonymous namespace)::DAGCombiner::visitMUL(llvm::SDNode*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3936:7
    #3 0x5caaafc in visit /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1633:40
    #4 0x5caaafc in (anonymous namespace)::DAGCombiner::combine(llvm::SDNode*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1748:10
    #5 0x5ca617e in Run /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1556:18
    #6 0x5ca617e in llvm::SelectionDAG::Combine(llvm::CombineLevel, llvm::AAResults*, llvm::CodeGenOpt::Level) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:23600:36
    #7 0x5e8d976 in llvm::SelectionDAGISel::CodeGenAndEmitDAG() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:811:13
    #8 0x5e8d374 in llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, true, false, void>, false, true>, llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, true, false, void>, false, true>, bool&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:727:3
    #9 0x5e8bc2f in llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1625:7
    #10 0x5e872ac in llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:509:3
    #11 0x51b564e in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:72:13
    #12 0x570c8ea in llvm::FPPassManager::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1439:27
    #13 0x571544d in llvm::FPPassManager::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1485:16
    #14 0x570d340 in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1554:27
    #15 0x570d340 in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:542:44
    #16 0x378bab6 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:687:8
    #17 0x37899ab in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:388:22
    #18 0x7f57aaa7309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #19 0x37629b9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc+0x37629b9)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12208:58 in 
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/AArch64/urem-seteq-nonzero.ll
--
********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (1):
  LLVM :: CodeGen/AArch64/urem-seteq-nonzero.ll
benshi001 updated this revision to Diff 370693.Sep 3 2021, 7:50 PM

@kda Could you please have a look at my new patch revision ? I have tested on my laptop (without sanitizer), which is slow to run full sanitizer tests.

The overflow is caused by int64_t multiplication overflow, which I have changed to class APInt.

kda added a comment.Sep 7 2021, 3:27 PM

@kda Could you please have a look at my new patch revision ? I have tested on my laptop (without sanitizer), which is slow to run full sanitizer tests.

Yes.
Appears to build correctly.
I apologize for the delay.