<div dir="ltr">FYI: reverted as r369236.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Aug 18, 2019 at 11:44 PM David Jones <<a href="mailto:dlj@google.com">dlj@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">FYI, this revision appears to cause some tests to fail under UBSAN.<div><br></div><div>Example test output:</div><div>llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll<br><div><br></div><div>[...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17: runtime error: load of value 168, which is not a valid value for type 'bool'<br> #0 [...] in AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_9::operator()(llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool) const [...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17<br> #1 [...] in void llvm::function_ref<void (llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool)>::callback_fn<AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_9>(long, llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool) [...]/llvm/include/llvm/ADT/STLExtras.h:125:12<br> #2 [...] in llvm::function_ref<void (llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool)>::operator()(llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool) const [...]/llvm/include/llvm/ADT/STLExtras.h:142:12<br> #3 [...] in bool genericValueTraversal<llvm::AAReturnedValues, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState>(llvm::Attributor&, llvm::IRPosition, llvm::AAReturnedValues const&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, llvm::function_ref<void (llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&, bool)> const&, int) [...]/llvm/lib/Transforms/IPO/Attributor.cpp:207:5<br> #4 [...] in AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::$_2::operator()(llvm::Value&, AAReturnedValuesImpl::updateImpl(llvm::Attributor&)::RVState&) const [...]/llvm/lib/Transforms/IPO/Attributor.cpp:902:12<br> #5 [...] in AAReturnedValuesImpl::updateImpl(llvm::Attributor&) [...]/llvm/lib/Transforms/IPO/Attributor.cpp:982:9<br> #6 [...] in llvm::AbstractAttribute::update(llvm::Attributor&) [...]/llvm/lib/Transforms/IPO/Attributor.cpp:264:16<br> #7 [...] in llvm::Attributor::run() [...]/llvm/lib/Transforms/IPO/Attributor.cpp:2543:17<br> #8 [...] in runAttributorOnModule(llvm::Module&) [...]/llvm/lib/Transforms/IPO/Attributor.cpp:2881:12<br> #9 [...] in (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) [...]/llvm/lib/IR/LegacyPassManager.cpp:1750:27<br> #10 [...] in llvm::legacy::PassManagerImpl::run(llvm::Module&) [...]/llvm/lib/IR/LegacyPassManager.cpp:1863:44<br> #11 [...] in main [...]/llvm/tools/opt/opt.cpp:892:12<br> #12 [...] in __libc_start_main (/usr/grte/v4/lib64/libc.so.6+[...])<br> #13 [...] in _start /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:108<br><br>SUMMARY: UndefinedBehaviorSanitizer: invalid-bool-load [...]/llvm/lib/Transforms/IPO/Attributor.cpp:891:17 in <br>FileCheck error: '-' is empty.<br></div><div><br></div><div><br></div></div><div><br></div><div>(The test llvm/test/Transforms/FunctionAttrs/arg_returned.ll fails in a similar way)</div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Aug 16, 2019 at 2:58 PM Johannes Doerfert via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: jdoerfert<br>
Date: Fri Aug 16 14:59:52 2019<br>
New Revision: 369160<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=369160&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=369160&view=rev</a><br>
Log:<br>
[Attributor] Fix: Do not partially resolve returned calls.<br>
<br>
By partially resolving returned calls we did not record that they were<br>
not fully resolved which caused odd behavior down the line. We could<br>
also end up with some, but not all, returned values of the callee in the<br>
returned values map of the caller, another odd behavior we want to<br>
avoid.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/Attributor.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/Attributor.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Attributor.cpp?rev=369160&r1=369159&r2=369160&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Attributor.cpp?rev=369160&r1=369159&r2=369160&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/Attributor.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/Attributor.cpp Fri Aug 16 14:59:52 2019<br>
@@ -943,12 +943,35 @@ ChangeStatus AAReturnedValuesImpl::updat<br>
<< static_cast<const AbstractAttribute &>(RetValAA)<br>
<< "\n");<br>
<br>
- // If we know something but not everyting about the returned values, keep<br>
- // track of that too. Hence, remember transitively unresolved calls.<br>
- UnresolvedCalls.insert(RetValAA.getUnresolvedCalls().begin(),<br>
- RetValAA.getUnresolvedCalls().end());<br>
+ // Do not try to learn partial information. If the callee has unresolved<br>
+ // return values we will treat the call as unresolved/opaque.<br>
+ auto &RetValAAUnresolvedCalls = RetValAA.getUnresolvedCalls();<br>
+ if (!RetValAAUnresolvedCalls.empty()) {<br>
+ UnresolvedCalls.insert(CB);<br>
+ continue;<br>
+ }<br>
+<br>
+ // Now check if we can track transitively returned values. If possible, thus<br>
+ // if all return value can be represented in the current scope, do so.<br>
+ bool Unresolved = false;<br>
+ for (auto &RetValAAIt : RetValAA.returned_values()) {<br>
+ Value *RetVal = RetValAAIt.first;<br>
+ if (isa<Argument>(RetVal) || isa<CallBase>(RetVal) ||<br>
+ isa<Constant>(RetVal))<br>
+ continue;<br>
+ // Anything that did not fit in the above categories cannot be resolved,<br>
+ // mark the call as unresolved.<br>
+ LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned value "<br>
+ "cannot be translated: "<br>
+ << *RetVal << "\n");<br>
+ UnresolvedCalls.insert(CB);<br>
+ Unresolved = true;<br>
+ break;<br>
+ }<br>
+<br>
+ if (Unresolved)<br>
+ continue;<br>
<br>
- // Now track transitively returned values.<br>
for (auto &RetValAAIt : RetValAA.returned_values()) {<br>
Value *RetVal = RetValAAIt.first;<br>
if (Argument *Arg = dyn_cast<Argument>(RetVal)) {<br>
@@ -967,12 +990,6 @@ ChangeStatus AAReturnedValuesImpl::updat<br>
NewRVsMap[RetVal].insert(It.second.begin(), It.second.end());<br>
continue;<br>
}<br>
- // Anything that did not fit in the above categories cannot be resolved,<br>
- // mark the call as unresolved.<br>
- LLVM_DEBUG(dbgs() << "[AAReturnedValues] transitively returned value "<br>
- "cannot be translated: "<br>
- << *RetVal << "\n");<br>
- UnresolvedCalls.insert(CB);<br>
}<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>