<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Feb 18, 2014, at 11:19 AM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Feb 18, 2014 at 11:00:05AM -0800, Quentin Colombet wrote:<br><blockquote type="cite"><br>On Feb 18, 2014, at 10:30 AM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">On Tue, Feb 18, 2014 at 09:47:21AM -0800, Quentin Colombet wrote:<br><blockquote type="cite"><br>On Feb 18, 2014, at 9:37 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br><blockquote type="cite">Hi Tom,<br><br>On Feb 18, 2014, at 9:21 AM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br><br><blockquote type="cite">Hi Quentin,<br><br>Sorry I missed this on the first time around:<span class="Apple-converted-space"> </span><br><br><blockquote type="cite">--- a/lib/Transforms/Scalar/CodeGenPrepare.cpp<br>+++ b/lib/Transforms/Scalar/CodeGenPrepare.cpp<br>@@ -1728,6 +1730,35 @@<br>TypePromotionHelper::promoteOperandForOther(Instruction *SExt,<br>return SExtOpnd;<br>}<br><br>+/// IsPromotionProfitable - Check whether or not promoting an<br>instruction<br>+/// to a wider type was profitable.<br>+/// \p MatchedSize gives the number of instructions that have been<br>matched<br>+/// in the addressing mode after the promotion was applied.<br>+/// \p SizeWithPromotion gives the number of created instructions for<br>+/// the promotion plus the number of instructions that have been<br>+/// matched in the addressing mode before the promotion.<br>+/// \p PromotedOperand is the value that has been promoted.<br>+/// \return True if the promotion is profitable, false otherwise.<br>+bool<br>+AddressingModeMatcher::IsPromotionProfitable(unsigned MatchedSize,<br>+ unsigned SizeWithPromotion,<br>+ Value *PromotedOperand) const {<br>+ // We folded less instructions than what we created to promote the operand.<br>+ // This is not profitable.<br>+ if (MatchedSize < SizeWithPromotion)<br>+ return false;<br>+ if (MatchedSize > SizeWithPromotion)<br>+ return true;<br>+ // The promotion is neutral but it may help folding the signextension in<br>+ // loads for instance.<br>+ // Check that we did not create an illegal instruction.<br>+ Instruction *PromotedInst = dyn_cast<Instruction>(PromotedOperand);<br>+ if (!PromotedInst)<br>+ return false;<br>+ return TLI.isOperationLegalOrCustom(PromotedInst->getOpcode(),<br>+ EVT::getEVT(PromotedInst->getType()));<br></blockquote><br>This won't work, because the IR Instructions and SelectionDAG Opcodes<br>have different enumeration values.<br></blockquote>Right, good catch!<br><br><blockquote type="cite">The original test case must have<br>passed, because it is using a default TargetLowering implementation<br>rather than the one from R600.<br><br>I've attached an updated version of the original test case which compiles<br>the program using llc and triggers the bug again.<br><br>I couldn't find other transforms that use the isOperation* callbacks, so<br>I think you may have to add a function to convert from IR Instruction to<br>SelectionDAG Opcodes or implement this some other way.<br></blockquote>I think a specific hook in TargetLowering may be the best approach here.<br>Let me see what I can come up with.<br>If you have ideas on the naming, let me know :).<br></blockquote>In fact, we may be able to use this:<br>int TargetLoweringBase::InstructionOpcodeToISD(unsigned Opcode)const<br></blockquote><br>Oh, I didn't see that. That should work.<br></blockquote>Almost there.<br>I am seeing a link error when creating the bugpoint executable.<br><br>llvm[2]: Linking Release executable bugpoint (without symbols)<br>Undefined symbols for architecture x86_64:<br> "llvm::TargetLoweringBase::InstructionOpcodeToISD(unsigned int) const", referenced from:<br> (anonymous namespace)::AddressingModeMatcher::MatchOperationAddr(llvm::User*, unsigned int, unsigned int, bool*) in libLLVMScalarOpts.a(CodeGenPrepare.o)<br>ld: symbol(s) not found for architecture x86_64<br><br>The thing that I do not understand is why the linker is not find this function, whereas it does not complain about TargetLoweringBase::isLegalAddressingMode.<br>I may have missed something obvious, but I do not see it.<br></blockquote><br>I'm not seeing this error on my machine. What build system are you<br>using?</div></blockquote><div>Makefile on Mac OS.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> Did you try running make clean first?<br></div></blockquote><div>I’ve tried but it does not seem to make the trick.</div><div><br></div><div>Could you try the attached patch?</div><div><br></div><div>Thanks,</div><div>-Quentin</div><div></div></div></body></html>