Patch for mem2reg pass

[David Blaikie]

On Fri, Jul 19, 2013 at 10:09 AM, Piotr Perek <pperek1234 at gmail.com> wrote:
> Hi David,
>
>
> On 17.07.2013 20:38, David Blaikie wrote:
>>
>> Sorry for the lack of details - you should be able to reproduce the
>> bug that caused me to add the conditional fix (the "if this thing is
>> in the map then do stuff" bit that wasn't in your original patch) by
>> applying your original patch (without my fix) and having compiler-rt
>> checked out beneath llvm/projects then running "make check-all" or the
>> equivalent target for your build system. This will build LLVM and
>> Clang, then using the just-built Clang it'll build compiler-rt (this
>> is where you'll see the crash) and if that passes, it'll run the LLVM,
>> Clang, and Compiler-rt lit tests.
>>
>> So what I'm hoping is you can reproduce the failure that lead to my
>> fix, reduce a test case so you can demonstrate that failure using the
>> llvm lit tests rather than using the compiler-rt build, then run the
>> same command (make check-all) again & hit the second issue that I
>> didn't get around to fixing yet, reduce a test case for that and fix
>> the issue... and if you're lucky, at that point you should be able to
>> get a "make check-all" to pass (unless there are other hidden issues
>> yet to be revealed).
>>
>> At that point you might want to try a self-host. Build Clang, then
>> build Clang with that Clang.
>>
>>> Best regards,
>>> Piotr
>
>
> Thank you for your detailed answer. I was able to reproduce failures that
> you found.

There was one failure I found & fixed (the lookup check), one I found
& didn't fix (the fix you added with the "first non-PHI" search), but
it seems there's (at least) one more. Something related to exception
handling and landing pads. I see this crash when running "make
check-all" with compiler-rt checked out. Are you not seeing this
failure? The stack looks something like:

#3  0x00007ffff69f8192 in __GI___assert_fail (assertion=0x33d92b6
<.L.str99> "isa<X>(Val) && \"cast<Ty>() argument of incompatible
type!\"", file=0x33d92f1 <.L.str99>
"src/include/llvm/Support/Casting.h", line=232, function=0x359215f
<.L__PRETTY_FUNCTION__._ZN4llvm4castINS_14LandingPadInstENS_14ilist_iteratorINS_11InstructionEEEEENS_10cast_rettyIT_T0_E8ret_typeERS7_>
"typename cast_retty<X, Y>::ret_type llvm::cast(Y &) [X =
llvm::LandingPadInst, Y = llvm::ilist_iterator<llvm::Instruction>]")
at assert.c:103
#4  0x0000000001691776 in llvm::cast<llvm::LandingPadInst,
llvm::ilist_iterator<llvm::Instruction> > (Val=...) at
src/include/llvm/Support/Casting.h:232
#5  0x00000000016910c9 in (anonymous
namespace)::InvokeInliningInfo::InvokeInliningInfo
(this=0x7fffffff9840, II=0x5c9db20) at
src/lib/Transforms/Utils/InlineFunction.cpp:67

I've attached the repro.

> I corrected this bugs and prepared additional test files. Please
> take a look.

Some of this code is still missing LLVM preferred formatting (* on the
right, { at the end of the line, rather than on a new line, etc). I've
reformatted it & simplified some code using existing APIs.

I've attached the updated patch. Also, I haven't looked at the test
cases yet but:

1) we don't put dates in test file names anymore
2) we try to cover more common/related test cases in one test file (to
keep the test run fast - spinning up separate processes is expensive).
If the three test cases can be covered easily in one file (in 3
functions, say), that would be ideal. Check out other tests
(especially those using the recently introduced CHECK-LABEL) for
examples.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: crash.tgz
Type: application/x-gzip
Size: 728646 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130729/5bc47c37/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mem2reg_2013_07_19.diff
Type: application/octet-stream
Size: 1759 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130729/5bc47c37/attachment.obj>