<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Hi David,</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">thanks for pointing this out to me. I would like to be able to reproduce this issue, is this visible in buildbot at all ?</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Thanks,</p>
<p style="margin-top:0;margin-bottom:0">Ranjeet</p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> David Jones <dlj@google.com><br>
<b>Sent:</b> 14 December 2018 19:22:17<br>
<b>To:</b> Ranjeet Singh<br>
<b>Cc:</b> llvm-commits@lists.llvm.org<br>
<b>Subject:</b> Re: [llvm] r348571 - [IR] Don't assume all functions are 4 byte aligned</font>
<div> </div>
</div>
<div>
<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="x_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="x_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>
</div>
</body>
</html>