Add ZOL to aie2p #311
Add ZOL to aie2p #311
Conversation
martien-de-jong
requested review from
abhinay-anubola,
abnikant,
andcarminati,
F-Stuckmann,
gbossu,
katerynamuts,
khallouh,
konstantinschwarz,
niwinanto,
SagarMaheshwari99 and
stephenneuendorffer
as code owners
martien-de-jong
force-pushed
the
martien.zol
branch
from
e7a3319 to
c83208c
Compare
| // LoopStart carries an immediate operand that is dedicated to the tripcount | ||
| // update of the pipeliner. We translate to ADD_NC, which has a similar | ||
| // operand. | ||
| BuildMI(*MBB, Start, Start->getDebugLoc(), TII->get(AIE2::ADD_NC), AIE2::LC) |
There was a problem hiding this comment.
This was not generic, but we only need the right numbers. They are now supplied by one single getZOLSupport() call.
martien-de-jong
force-pushed
the
martien.zol
branch
from
c83208c to
21a95b9
Compare
| LLVMContext &C = L->getHeader()->getContext(); | ||
| HWLoopInfo.CountType = Type::getInt32Ty(C); | ||
| HWLoopInfo.LoopDecrement = ConstantInt::get(HWLoopInfo.CountType, 1); | ||
| // We always allow nested hardware loops, but only the innermost loop |
There was a problem hiding this comment.
Yeah right. Note how we only allow single block loops above. Details.
| @@ -221,35 +245,31 @@ struct AIEBaseInstrInfo : public TargetInstrInfo { | |||
| // Opcodes related to hardware loop handling | |||
| virtual bool isHardwareLoopDec(unsigned Opcode) const { return false; } | |||
| virtual bool isHardwareLoopJNZ(unsigned Opcode) const { return false; } | |||
| virtual bool isHardwareLoopStart(unsigned Opcode) const { return false; } | |||
| virtual bool isHardwareLoopEnd(unsigned Opcode) const { return false; } | |||
| virtual bool isHardwareLoopStart(unsigned Opcode) const; | |||
There was a problem hiding this comment.
This completely stands on the base class now. I hope no need to override this in the future ;-)
There was a problem hiding this comment.
Yes. The base class will work if there's a fixed set of opcodes that is used in all occurrences. I expect this to remain true.
| HWLoopInfo.CounterInReg = true; | ||
| } | ||
| return true; | ||
| } |
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| // (c) Copyright 2023-2025 Advanced Micro Devices, Inc. or its affiliates |
There was a problem hiding this comment.
Should this be only 2025?
There was a problem hiding this comment.
No. This file was copied, then modified.
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "AIE2PTargetTransformInfo.h" |
There was a problem hiding this comment.
For the future (really not now): refactor this to a common class.
| extern cl::opt<unsigned> MaxUnrollCost; | ||
| extern cl::opt<unsigned> PreferSwpOverUnroll; | ||
|
|
||
| void AIE2PTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE, |
There was a problem hiding this comment.
Not really needed by this PR, but a nice-to-have change.
There was a problem hiding this comment.
I got it for free, and it made getting an end-to-end postpiperliner test working a lot eaesier.
|
General comment: looks good, just some outdated dates. |
martien-de-jong
force-pushed
the
martien.zol
branch
from
21a95b9 to
d0caf34
Compare
| Result.LoopEndOpcode = AIE2::PseudoLoopEnd; | ||
| Result.SetLoopCountOpcode = AIE2::ADD_NC; | ||
| Result.SetAddressOpcode = AIE2::MOVXM_lng_cg; | ||
| Result.LoopSetupDistance = 7; |
There was a problem hiding this comment.
Nit: Would be nice if you could comment on this number.
| @@ -329,6 +329,14 @@ bool AIEBaseInstrInfo::isCallBundle(MachineBasicBlock::iterator MII) const { | |||
| return IsReturnAddr; | |||
| } | |||
|
|
|||
| bool AIEBaseInstrInfo::isZOLTripCountDef(const MachineInstr &MI, | |||
There was a problem hiding this comment.
Nit: May be you dont need this to be virtual function anymore.
There was a problem hiding this comment.
Murphy law, in the formulation "Constants aren't, variables won't": if I leave it I will never need it; if I remove it, I will need it next week.
| bool AIEBaseInstrInfo::isZeroOverheadLoopSetupInstr( | ||
| const MachineInstr &MI) const { | ||
| auto ZOLSupport = getZOLSupport(); | ||
| if (!ZOLSupport) { |
There was a problem hiding this comment.
cant we just assert like in getLoopSetupDistance
There was a problem hiding this comment.
No. We are a simple predicate that returns whether this is a ZOLsetup instruction. If we haven't got ZOLSupport, it isn't one. No reason to abort.
| %12:_(s32) = nsw G_ADD %4, %11 | ||
| G_STORE %12(s32), %0(p0) :: (store (s32)) | ||
| %14:_(s32) = nuw nsw G_ADD %5, %13 | ||
| %15:_(s1) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.loop.decrement), %13(s32) |
There was a problem hiding this comment.
Do we need this entire loop to test this? can we simply use %15:_(s1) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.loop.decrement), %13(s32)
select_BRCOND cs has been refactored to BaseInstructionSelector
Put it in a separate source file
Cleanup of includes
martien-de-jong
force-pushed
the
martien.zol
branch
from
d0caf34 to
e56871b
Compare
|
@martien-de-jong I can see many ZOL tests for aie2 |
Leveraged the great similarity between aie2 and aie2p. All of ZOL can now be defined by a dozen of opcodes, registers and other numbers