<div dir="ltr"><div dir="ltr">Hello,<div><br></div><div>I'm seeing test failures with this revision under MSan. The tests are failing on use-of-uninitialized-value in ffmpeg.</div><div><br></div><div>You mentioned in the review that you had previously observed sanitizer issues. Does this sound related? If it's not clear, would you be amenable to reverting this change?</div><div><br></div><div>I did cleanly bisect the failure to this revision, so I'm quite confident that something in this revision triggers the failure mode. (It may be a pre-existing bug, though, not necessarily an issue in your changes.)</div><div><br></div><div>--dlj</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 7, 2018 at 12:37 AM Ranjeet Singh via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">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: rsingh<br>
Date: Fri Dec 7 00:34:59 2018<br>
New Revision: 348571<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=348571&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=348571&view=rev</a><br>
Log:<br>
[IR] Don't assume all functions are 4 byte aligned<br>
<br>
In some cases different alignments for function might be used to save<br>
space e.g. thumb mode with -Oz will try to use 2 byte function<br>
alignment. Similar patch that fixed this in other areas exists here<br>
<a href="https://reviews.llvm.org/D46110" rel="noreferrer" target="_blank">https://reviews.llvm.org/D46110</a><br>
<br>
This was approved previously <a href="https://reviews.llvm.org/D55115" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55115</a> (r348215)<br>
but when committed it caused failures on the sanitizer buildbots when<br>
building llvm with clang (containing this patch). This is now fixed<br>
because I've added a check to see if getting the parent module returns<br>
null if it does then set the alignment to 0.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D55115" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55115</a><br>
<br>
<br>
<br>
Added:<br>
llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll<br>
Removed:<br>
llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll<br>
Modified:<br>
llvm/trunk/lib/IR/ConstantFold.cpp<br>
<br>
Modified: llvm/trunk/lib/IR/ConstantFold.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=348571&r1=348570&r2=348571&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=348571&r1=348570&r2=348571&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/ConstantFold.cpp (original)<br>
+++ llvm/trunk/lib/IR/ConstantFold.cpp Fri Dec 7 00:34:59 2018<br>
@@ -27,6 +27,7 @@<br>
#include "llvm/IR/GlobalAlias.h"<br>
#include "llvm/IR/GlobalVariable.h"<br>
#include "llvm/IR/Instructions.h"<br>
+#include "llvm/IR/Module.h"<br>
#include "llvm/IR/Operator.h"<br>
#include "llvm/IR/PatternMatch.h"<br>
#include "llvm/Support/ErrorHandling.h"<br>
@@ -1077,10 +1078,10 @@ Constant *llvm::ConstantFoldBinaryInstru<br>
isa<GlobalValue>(CE1->getOperand(0))) {<br>
GlobalValue *GV = cast<GlobalValue>(CE1->getOperand(0));<br>
<br>
- // Functions are at least 4-byte aligned.<br>
- unsigned GVAlign = GV->getAlignment();<br>
- if (isa<Function>(GV))<br>
- GVAlign = std::max(GVAlign, 4U);<br>
+ unsigned GVAlign =<br>
+ GV->getParent()<br>
+ ? GV->getPointerAlignment(GV->getParent()->getDataLayout())<br>
+ : 0;<br>
<br>
if (GVAlign > 1) {<br>
unsigned DstWidth = CI2->getType()->getBitWidth();<br>
<br>
Added: llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll?rev=348571&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll?rev=348571&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll (added)<br>
+++ llvm/trunk/test/Analysis/ConstantFolding/func-and-folding.ll Fri Dec 7 00:34:59 2018<br>
@@ -0,0 +1,27 @@<br>
+; RUN: opt < %s -constprop -S -o - | FileCheck %s<br>
+<br>
+; Function Attrs: minsize norecurse nounwind optsize readnone<br>
+define dso_local void @foo1() #0 {<br>
+entry:<br>
+ ret void<br>
+}<br>
+<br>
+; Function Attrs: minsize norecurse nounwind optsize readnone<br>
+define dso_local void @foo2() align 4 {<br>
+entry:<br>
+ ret void<br>
+}<br>
+<br>
+; Function Attrs: minsize nounwind optsize<br>
+define dso_local i32 @main() local_unnamed_addr #1 {<br>
+entry:<br>
+; CHECK: ptrtoint<br>
+ %call = tail call i32 bitcast (i32 (...)* @process to i32 (i32)*)(i32 and (i32 ptrtoint (void ()* @foo1 to i32), i32 2)) #3<br>
+; CHECK-NEXT: ptrtoint<br>
+ %call2 = tail call i32 bitcast (i32 (...)* @process to i32 (i32)*)(i32 and (i32 ptrtoint (void ()* @foo2 to i32), i32 2)) #3<br>
+ ret i32 0<br>
+}<br>
+<br>
+; Function Attrs: minsize optsize<br>
+declare dso_local i32 @process(...) local_unnamed_addr #2<br>
+<br>
<br>
Removed: llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll?rev=348570&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll?rev=348570&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll (original)<br>
+++ llvm/trunk/test/Assembler/2004-03-07-FunctionAddressAlignment.ll (removed)<br>
@@ -1,16 +0,0 @@<br>
-; RUN: llvm-as < %s | llvm-dis | not grep ptrtoint<br>
-; RUN: verify-uselistorder %s<br>
-; All of these should be eliminable<br>
-<br>
-<br>
-define i32 @foo() {<br>
- ret i32 and (i32 ptrtoint (i32()* @foo to i32), i32 1)<br>
-}<br>
-<br>
-define i32 @foo2() {<br>
- ret i32 and (i32 1, i32 ptrtoint (i32()* @foo2 to i32))<br>
-}<br>
-<br>
-define i1 @foo3() {<br>
- ret i1 icmp ne (i1()* @foo3, i1()* null)<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="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>