<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 15, 2016, at 10:32 AM, Keno Fischer <<a href="mailto:kfischer@college.harvard.edu" class="">kfischer@college.harvard.edu</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">I can look into this. The confusion of representing <span style="font-size:12.8px" class="">x86_fp80 in the debug info is an issue which I have have brought up separately on the mailing list. I'm confused about this particular case though. What is happening? Is it splitting an array of </span><span style="font-size:12.8px" class="">x86_fp80 and getting confused about the size? I think the proper fix is probably to adjust the size information for the piece. I'd recommend against reverting this patch, since you'll end up with crashes in the backend due to incorrect IR. If you would like, I think a possible course of action would be to temporarily remove the assert, which I think should only lead to incorrect debug info.</span></div></div></blockquote><div><br class=""></div><div>Removing the assertion (until we found a better fix) seems reasonable to me. I double-checked that there isn't also a similar assertion in the back end, so having a single piece that covers the entire value will only cause it to choose a less efficient (but valid) DWARF representation.</div><div>Note that the backend will assert on overlapping pieces, but if I understand the problem correctly in this case the x86_fp80 isn't split into two values, it's resized from 128 to 80 bits without there being a second piece covering bits 80-127?</div><div><br class=""></div><div>-- adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Mon, Aug 15, 2016 at 6:51 AM, James Molloy <span dir="ltr" class=""><<a href="mailto:james@jamesmolloy.co.uk" target="_blank" class="">james@jamesmolloy.co.uk</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="">Hi,<div class=""><br class=""></div><div class="">Looking further at this, the issue stems from the type involved, which is x86_fp80. This type has a n AllocSize of 128 bits but an "actual" size of 80 bits. Part of SROA is using the alloc size and another part is using the non-alloc size which is why we get a size disparity.</div><div class=""><br class=""></div><div class="">I've put assertions in to this code that fire when DL.getTypeSizeInBits != DL.getTypeAllocSizeInBits, and those never fire in the test suite. </div><div class=""><br class=""></div><div class="">As this patch isn't sufficiently tested, I vote that we revert this patch.</div><div class=""><br class=""></div><div class="">Thoughts?</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">James</div><div class=""><div class="h5"><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, 15 Aug 2016 at 10:52 James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank" class="">james@jamesmolloy.co.uk</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><br class="">Hi Keno, Adrian,<div class=""><br class=""></div><div class="">A recent change of mine to SimplifyCFG exposed a bug in this commit. This assert is firing:</div><div class=""><br class=""></div><div class=""><div class=""> assert(Pieces.size() == 1 &&</div></div></div><div dir="ltr" class=""><div class=""><div class=""> "partition is as large as original alloca");</div></div></div><div dir="ltr" class=""><div class=""></div><div class=""><br class=""></div><div class="">From the phab review it looks like this assert was added in late in the review process and my testing shows that this assert is never tested at all in the regression tests (changing to assert(0) doesn't cause any failures!)</div><div class=""><br class=""></div><div class="">The error is shown here: <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/23920/steps/annotate/logs/stdio" target="_blank" class="">http://lab.llvm.org:<wbr class="">8011/builders/sanitizer-x86_<wbr class="">64-linux-autoconf/builds/<wbr class="">23920/steps/annotate/logs/<wbr class="">stdio</a></div><div class=""><br class=""></div><div class="">It's easily reproducible with ToT clang simply compiling the file projects/compiler-rt/lib/<wbr class="">builtins/powixf2.c *with debug info enabled (-g)*.</div><div class=""><br class=""></div><div class="">I'd fix it myself but after staring for a while I can't work out if the the assert sense is wrong (the text "partition is as large as original alloca" seems like a failure mode to me, which would indicate that perhaps the test should have been "Pieces.size() != 1" ?) or if the assert is correct but there is a failure somewhere else.</div><div class=""><br class=""></div><div class="">Could you please take a look?</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">James</div></div><div dir="ltr" class=""><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Thu, 14 Jan 2016 at 20:10 Keno Fischer via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kfischer<br class="">
Date: Thu Jan 14 14:06:34 2016<br class="">
New Revision: 257795<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=257795&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project?rev=257795&view=rev</a><br class="">
Log:<br class="">
[SROA] Also insert a bit piece expression if only one piece is needed<br class="">
<br class="">
Summary: If SROA creates only one piece (e.g. because the other is not needed),<br class="">
it still needs to create a bit_piece expression if that bit piece is smaller<br class="">
than the original size of the alloca.<br class="">
<br class="">
Reviewers: aprantl<br class="">
<br class="">
Subscribers: llvm-commits<br class="">
<br class="">
Differential Revision: <a href="http://reviews.llvm.org/D16187" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D16187</a><br class="">
<br class="">
Added:<br class="">
llvm/trunk/test/Transforms/<wbr class="">SROA/dbg-single-piece.ll<br class="">
Modified:<br class="">
llvm/trunk/lib/Transforms/<wbr class="">Scalar/SROA.cpp<br class="">
<br class="">
Modified: llvm/trunk/lib/Transforms/<wbr class="">Scalar/SROA.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=257795&r1=257794&r2=257795&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/llvm/trunk/lib/<wbr class="">Transforms/Scalar/SROA.cpp?<wbr class="">rev=257795&r1=257794&r2=<wbr class="">257795&view=diff</a><br class="">
==============================<wbr class="">==============================<wbr class="">==================<br class="">
--- llvm/trunk/lib/Transforms/<wbr class="">Scalar/SROA.cpp (original)<br class="">
+++ llvm/trunk/lib/Transforms/<wbr class="">Scalar/SROA.cpp Thu Jan 14 14:06:34 2016<br class="">
@@ -4024,12 +4024,12 @@ bool SROA::splitAlloca(AllocaInst &AI, A<br class="">
auto *Var = DbgDecl->getVariable();<br class="">
auto *Expr = DbgDecl->getExpression();<br class="">
DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);<br class="">
- bool IsSplit = Pieces.size() > 1;<br class="">
+ uint64_t AllocaSize = DL.getTypeSizeInBits(AI.<wbr class="">getAllocatedType());<br class="">
for (auto Piece : Pieces) {<br class="">
// Create a piece expression describing the new partition or reuse AI's<br class="">
// expression if there is only one partition.<br class="">
auto *PieceExpr = Expr;<br class="">
- if (IsSplit || Expr->isBitPiece()) {<br class="">
+ if (Piece.Size < AllocaSize || Expr->isBitPiece()) {<br class="">
// If this alloca is already a scalar replacement of a larger aggregate,<br class="">
// Piece.Offset describes the offset inside the scalar.<br class="">
uint64_t Offset = Expr->isBitPiece() ? Expr->getBitPieceOffset() : 0;<br class="">
@@ -4043,6 +4043,9 @@ bool SROA::splitAlloca(AllocaInst &AI, A<br class="">
Size = std::min(Size, AbsEnd - Start);<br class="">
}<br class="">
PieceExpr = DIB.createBitPieceExpression(<wbr class="">Start, Size);<br class="">
+ } else {<br class="">
+ assert(Pieces.size() == 1 &&<br class="">
+ "partition is as large as original alloca");<br class="">
}<br class="">
<br class="">
// Remove any existing dbg.declare intrinsic describing the same alloca.<br class="">
<br class="">
Added: llvm/trunk/test/Transforms/<wbr class="">SROA/dbg-single-piece.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/dbg-single-piece.ll?rev=257795&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/llvm/trunk/test/<wbr class="">Transforms/SROA/dbg-single-<wbr class="">piece.ll?rev=257795&view=auto</a><br class="">
==============================<wbr class="">==============================<wbr class="">==================<br class="">
--- llvm/trunk/test/Transforms/<wbr class="">SROA/dbg-single-piece.ll (added)<br class="">
+++ llvm/trunk/test/Transforms/<wbr class="">SROA/dbg-single-piece.ll Thu Jan 14 14:06:34 2016<br class="">
@@ -0,0 +1,37 @@<br class="">
+; RUN: opt -sroa %s -S | FileCheck %s<br class="">
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:<wbr class="">32:64-S128"<br class="">
+<br class="">
+%foo = type { [8 x i8], [8 x i8] }<br class="">
+<br class="">
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0<br class="">
+define void @_<wbr class="">ZL18findInsertLocationPN4llvm1<wbr class="">7MachineBasicBlockENS_<wbr class="">9SlotIndexERNS_<wbr class="">13LiveIntervalsE() {<br class="">
+entry:<br class="">
+ %retval = alloca %foo, align 8<br class="">
+ call void @llvm.dbg.declare(metadata %foo* %retval, metadata !1, metadata !7), !dbg !8<br class="">
+; Checks that SROA still inserts a bit_piece expression, even if it produces only one piece<br class="">
+; (as long as that piece is smaller than the whole thing)<br class="">
+; CHECK-NOT: call void @llvm.dbg.value<br class="">
+; CHECK: call void @llvm.dbg.value(metadata %foo* undef, i64 0, metadata !1, metadata ![[BIT_PIECE:[0-9]+]]), !dbg<br class="">
+; CHECK-NOT: call void @llvm.dbg.value<br class="">
+; CHECK: ![[BIT_PIECE]] = !DIExpression(DW_OP_bit_piece, 64, 64)<br class="">
+ %0 = bitcast %foo* %retval to i8*<br class="">
+ %1 = getelementptr inbounds i8, i8* %0, i64 8<br class="">
+ %2 = bitcast i8* %1 to %foo**<br class="">
+ store %foo* undef, %foo** %2, align 8<br class="">
+ ret void<br class="">
+}<br class="">
+<br class="">
+attributes #0 = { nounwind readnone }<br class="">
+<br class="">
+!<a href="http://llvm.dbg.cu/" rel="noreferrer" target="_blank" class="">llvm.dbg.cu</a> = !{}<br class="">
+!llvm.module.flags = !{!0}<br class="">
+<br class="">
+!0 = !{i32 2, !"Debug Info Version", i32 3}<br class="">
+!1 = !DILocalVariable(name: "I", scope: !2, file: !3, line: 947, type: !4)<br class="">
+!2 = distinct !DISubprogram(name: "findInsertLocation", linkageName: "_<wbr class="">ZL18findInsertLocationPN4llvm1<wbr class="">7MachineBasicBlockENS_<wbr class="">9SlotIndexERNS_<wbr class="">13LiveIntervalsE", scope: !3, file: !3, line: 937, isLocal: true, isDefinition: true, scopeLine: 938, flags: DIFlagPrototyped, isOptimized: true)<br class="">
+!3 = !DIFile(filename: "none", directory: ".")<br class="">
+!4 = !DICompositeType(tag: DW_TAG_class_type, name: "bundle_iterator<llvm::<wbr class="">MachineInstr, llvm::ilist_iterator<llvm::<wbr class="">MachineInstr> >", scope: !5, file: !3, line: 163, size: 128, align: 64, elements: !6, templateParams: !6, identifier: "_<wbr class="">ZTSN4llvm17MachineBasicBlock15<wbr class="">bundle_iteratorINS_<wbr class="">12MachineInstrENS_14ilist_<wbr class="">iteratorIS2_EEEE")<br class="">
+!5 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "MachineBasicBlock", file: !3, line: 68, size: 1408, align: 64, identifier: "_<wbr class="">ZTSN4llvm17MachineBasicBlockE"<wbr class="">)<br class="">
+!6 = !{}<br class="">
+!7 = !DIExpression()<br class="">
+!8 = !DILocation(line: 947, column: 35, scope: !2)<br class="">
<br class="">
<br class="">
______________________________<wbr class="">_________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div></div></div></blockquote></div></div></div></div>
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></body></html>