<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">I took a different approach and just don’t record those spills for the hoisting algo.<div class="">This is <span style="font-family: Menlo; font-size: 11px; background-color: rgb(255, 255, 255);" class="">r304850</span></div><div class=""><font face="Menlo" class=""><span style="font-size: 11px; background-color: rgb(255, 255, 255);" class=""><br class=""></span></font><div class=""><div class=""><div>Thanks<br class=""><blockquote type="cite" class=""><div class="">On Jun 6, 2017, at 2:02 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">In theory, yes, but I don't think it is likely to matter. That would mean we spill undefs on several paths.<br class=""><br class="">My main concern is how to write a test case for this.<br class=""><br class="">I'll have a look.<br class=""><br class=""><blockquote type="cite" class="">Le 5 juin 2017 à 17:05, Matthias Braun <<a href="mailto:mbraun@apple.com" class="">mbraun@apple.com</a>> a écrit :<br class=""><br class="">InlineSpiller::hoistAllSpills() and hoistSpillInsideBB() seem to use storeRegToStackSlot() directly instead of using insertSpill() do they need to be adapted as well?<br class=""><br class="">- Matthias<br class=""><br class=""><blockquote type="cite" class="">On Jun 5, 2017, at 4:51 PM, Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Author: qcolombet<br class="">Date: Mon Jun 5 18:51:27 2017<br class="">New Revision: 304752<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304752&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=304752&view=rev</a><br class="">Log:<br class="">[InlineSpiller] Don't spill fully undef values<br class=""><br class="">Althought it is not wrong to spill undef values, it is useless and harms<br class="">both code size and runtime. Before spilling a value, check that its<br class="">content actually matters.<br class=""><br class=""><a href="http://www.llvm.org/PR33311" class="">http://www.llvm.org/PR33311</a><br class=""><br class="">Added:<br class=""> llvm/trunk/test/CodeGen/AArch64/spill-undef.mir<br class="">Modified:<br class=""> llvm/trunk/lib/CodeGen/InlineSpiller.cpp<br class=""><br class="">Modified: llvm/trunk/lib/CodeGen/InlineSpiller.cpp<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/InlineSpiller.cpp?rev=304752&r1=304751&r2=304752&view=diff<br class="">==============================================================================<br class="">--- llvm/trunk/lib/CodeGen/InlineSpiller.cpp (original)<br class="">+++ llvm/trunk/lib/CodeGen/InlineSpiller.cpp Mon Jun 5 18:51:27 2017<br class="">@@ -857,14 +857,36 @@ void InlineSpiller::insertReload(unsigne<br class=""> ++NumReloads;<br class="">}<br class=""><br class="">+/// Check if \p Def fully defines a VReg with an undefined value.<br class="">+/// If that's the case, that means the value of VReg is actually<br class="">+/// not relevant.<br class="">+static bool isFullUndefDef(const MachineInstr &Def) {<br class="">+ if (!Def.isImplicitDef())<br class="">+ return false;<br class="">+ assert(Def.getNumOperands() == 1 &&<br class="">+ "Implicit def with more than one definition");<br class="">+ // We can say that the VReg defined by Def is undef, only if it is<br class="">+ // fully defined by Def. Otherwise, some of the lanes may not be<br class="">+ // undef and the value of the VReg matters.<br class="">+ return !Def.getOperand(0).getSubReg();<br class="">+}<br class="">+<br class="">/// insertSpill - Insert a spill of NewVReg after MI.<br class="">void InlineSpiller::insertSpill(unsigned NewVReg, bool isKill,<br class=""> MachineBasicBlock::iterator MI) {<br class=""> MachineBasicBlock &MBB = *MI->getParent();<br class=""><br class=""> MachineInstrSpan MIS(MI);<br class="">- TII.storeRegToStackSlot(MBB, std::next(MI), NewVReg, isKill, StackSlot,<br class="">- MRI.getRegClass(NewVReg), &TRI);<br class="">+ if (isFullUndefDef(*MI))<br class="">+ // Don't spill undef value.<br class="">+ // Anything works for undef, in particular keeping the memory<br class="">+ // uninitialized is a viable option and it saves code size and<br class="">+ // run time.<br class="">+ BuildMI(MBB, std::next(MI), MI->getDebugLoc(), TII.get(TargetOpcode::KILL))<br class="">+ .addReg(NewVReg, getKillRegState(isKill));<br class="">+ else<br class="">+ TII.storeRegToStackSlot(MBB, std::next(MI), NewVReg, isKill, StackSlot,<br class="">+ MRI.getRegClass(NewVReg), &TRI);<br class=""><br class=""> LIS.InsertMachineInstrRangeInMaps(std::next(MI), MIS.end());<br class=""><br class=""><br class="">Added: llvm/trunk/test/CodeGen/AArch64/spill-undef.mir<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/spill-undef.mir?rev=304752&view=auto<br class="">==============================================================================<br class="">--- llvm/trunk/test/CodeGen/AArch64/spill-undef.mir (added)<br class="">+++ llvm/trunk/test/CodeGen/AArch64/spill-undef.mir Mon Jun 5 18:51:27 2017<br class="">@@ -0,0 +1,67 @@<br class="">+# RUN: llc %s -run-pass greedy -o - | FileCheck %s<br class="">+# Check that we don't insert spill code for undef values.<br class="">+# Uninitialized memory for them is fine.<br class="">+# PR33311<br class="">+--- |<br class="">+ ; ModuleID = 'stuff.ll'<br class="">+ target triple = "aarch64--"<br class="">+ <br class="">+ @g = external global i32<br class="">+ <br class="">+ define void @foobar() {<br class="">+ ret void<br class="">+ }<br class="">+ <br class="">+...<br class="">+---<br class="">+name: foobar<br class="">+alignment: 2<br class="">+tracksRegLiveness: true<br class="">+registers: <br class="">+ - { id: 0, class: gpr32 }<br class="">+ - { id: 1, class: gpr32 }<br class="">+ - { id: 2, class: gpr32all }<br class="">+ - { id: 3, class: gpr32 }<br class="">+ - { id: 4, class: gpr64common }<br class="">+ - { id: 5, class: gpr32 }<br class="">+ - { id: 6, class: gpr64common }<br class="">+ - { id: 7, class: gpr32 }<br class="">+ - { id: 8, class: gpr32 }<br class="">+ - { id: 9, class: gpr64 }<br class="">+body: |<br class="">+ bb.0:<br class="">+ liveins: %x0<br class="">+ successors: %bb.1, %bb.2<br class="">+<br class="">+ ; %8 is going to be spilled.<br class="">+ ; But on that path, we don't care about its value.<br class="">+ ; Emit a simple KILL instruction instead of an<br class="">+ ; actual spill.<br class="">+ ; CHECK: [[UNDEF:%[0-9]+]] = IMPLICIT_DEF<br class="">+ ; CHECK-NEXT: KILL [[UNDEF]]<br class="">+ %8 = IMPLICIT_DEF<br class="">+ ; %9 us going to be spilled.<br class="">+ ; But it is only partially undef.<br class="">+ ; Make sure we spill it properly<br class="">+ ; CHECK: [[NINE:%[0-9]+]] = COPY %x0<br class="">+ ; CHECK: [[NINE]].sub_32 = IMPLICIT_DEF<br class="">+ ; CHECK-NEXT: STRXui [[NINE]]<br class="">+ %9 = COPY %x0<br class="">+ %9.sub_32 = IMPLICIT_DEF<br class="">+ CBNZW %wzr, %bb.2<br class="">+ B %bb.1<br class="">+ <br class="">+ bb.1:<br class="">+ %4 = ADRP target-flags(aarch64-page) @g<br class="">+ %8 = LDRWui %4, target-flags(aarch64-pageoff, aarch64-nc) @g :: (volatile dereferenceable load 4 from @g)<br class="">+ INLINEASM $nop, 1, 12, implicit-def dead early-clobber %x0, 12, implicit-def dead early-clobber %x1, 12, implicit-def dead early-clobber %x2, 12, implicit-def dead early-clobber %x3, 12, implicit-def dead early-clobber %x4, 12, implicit-def dead early-clobber %x5, 12, implicit-def dead early-clobber %x6, 12, implicit-def dead early-clobber %x7, 12, implicit-def dead early-clobber %x8, 12, implicit-def dead early-clobber %x9, 12, implicit-def dead early-clobber %x10, 12, implicit-def dead early-clobber %x11, 12, implicit-def dead early-clobber %x12, 12, implicit-def dead early-clobber %x13, 12, implicit-def dead early-clobber %x14, 12, implicit-def dead early-clobber %x15, 12, implicit-def dead early-clobber %x16, 12, implicit-def dead early-clobber %x17, 12, implicit-def dead early-clobber %x18, 12, implicit-def dead early-clobber %x19, 12, implicit-def dead early-clobber %x20, 12, implicit-def dead early-clobber %x21, 12, implicit-def dead early-clobber %x22, 12, implicit-def d<br class="">ead early-clobber %x23, 12, implicit-def dead early-clobber %x24, 12, implicit-def dead early-clobber %x25, 12, implicit-def dead early-clobber %x26, 12, implicit-def dead early-clobber %x27, 12, implicit-def dead early-clobber %x28, 12, implicit-def dead early-clobber %fp, 12, implicit-def dead early-clobber %lr<br class="">+ <br class="">+ bb.2:<br class="">+ INLINEASM $nop, 1, 12, implicit-def dead early-clobber %x0, 12, implicit-def dead early-clobber %x1, 12, implicit-def dead early-clobber %x2, 12, implicit-def dead early-clobber %x3, 12, implicit-def dead early-clobber %x4, 12, implicit-def dead early-clobber %x5, 12, implicit-def dead early-clobber %x6, 12, implicit-def dead early-clobber %x7, 12, implicit-def dead early-clobber %x8, 12, implicit-def dead early-clobber %x9, 12, implicit-def dead early-clobber %x10, 12, implicit-def dead early-clobber %x11, 12, implicit-def dead early-clobber %x12, 12, implicit-def dead early-clobber %x13, 12, implicit-def dead early-clobber %x14, 12, implicit-def dead early-clobber %x15, 12, implicit-def dead early-clobber %x16, 12, implicit-def dead early-clobber %x17, 12, implicit-def dead early-clobber %x18, 12, implicit-def dead early-clobber %x19, 12, implicit-def dead early-clobber %x20, 12, implicit-def dead early-clobber %x21, 12, implicit-def dead early-clobber %x22, 12, implicit-def d<br class="">ead early-clobber %x23, 12, implicit-def dead early-clobber %x24, 12, implicit-def dead early-clobber %x25, 12, implicit-def dead early-clobber %x26, 12, implicit-def dead early-clobber %x27, 12, implicit-def dead early-clobber %x28, 12, implicit-def dead early-clobber %fp, 12, implicit-def dead early-clobber %lr<br class="">+ %6 = ADRP target-flags(aarch64-page) @g<br class="">+ %w0 = MOVi32imm 42<br class="">+ STRWui %8, %6, target-flags(aarch64-pageoff, aarch64-nc) @g :: (volatile store 4 into @g)<br class="">+ STRXui %9, %6, target-flags(aarch64-pageoff, aarch64-nc) @g :: (volatile store 8 into @g)<br class="">+ RET_ReallyLR implicit killed %w0<br class="">+<br class="">+...<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class="">llvm-commits@lists.llvm.org<br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></blockquote><br class=""></blockquote></div></div></blockquote></div><br class=""></div></div></div></body></html>