[llvm] r226490 - IR: Remove direct comparisons against Metadata::Storage, NFC

[Timur Iskhodzhanov]

On Tue Jan 20 2015 at 6:20:34 PM Duncan Exon Smith <dexonsmith at apple.com>
wrote:

>
>
> On Jan 20, 2015, at 3:48 AM, Timur Iskhodzhanov <timurrrr at google.com>
> wrote:
>
> Hi Duncan,
>
> This has broken down "ninja check-asan" on 32-bit Windows.
> Assertion failed: Context.hasReplaceableUses() && "Expected RAUW support",
> file C:\src\llvm_bot\slave\win\build\llvm\include\llvm/IR/Metadata.h,
> line 976
>
>
> Can you include the asan output?   A backtrace?   Point at a buildbot?   I
> don't have the tools to reproduce this.
>

There's no public buildbot for ASan itself, but turns out this problem is
not even specific to ASan.

This public bot just builds Chromium with Clang
http://build.chromium.org/p/chromium.fyi/builders/CrWinClang/builds/1295/steps/compile/logs/stdio


Assertion failed: isTemporary() && "Expected temporary node", file
C:\b\build\slave\CrWinClang\build\src\third_party\llvm\include\llvm/IR/Metadata.h,
line 741

since r226499.

OK, I'll try to minimize...
(I suppose the change looks sane at the first glance and you can't come up
with a repro yourself)


> It seems to be hard to revert now...
>
> You've committed a lot of small changes in a very short timeframe -
>
>
> Actually, I committed work over the span of an entire work day.
>

Oh, I see.

> - I assume you're using git?  Is there a reason why you don't squash your
> changes before committing?  It makes finding bad commits / reverting
> harder...
>
>
> Besides being developer policy to separate changes as much as possible, it
> also makes changes easier to review, and isolating which change causes a
> problem easier to determine.
>

Unless it results in complex history and back-in-forth changes, I suppose.
I've spent more than an hour today trying to locally bisect and revert the
changes and ended up just locally reverting 40+ of your commits after
226490 to make progress.  Unfortunately, reverting just r226490 results in
a waterfall of merge conflicts.

Do you think it's reasonable to temporarily revert your changes in the
[r226490, HEAD] range to unbreak stuff?
I have a patch ready for that.

> On Mon Jan 19 2015 at 10:30:36 PM Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>> Author: dexonsmith
>> Date: Mon Jan 19 13:26:24 2015
>> New Revision: 226490
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=226490&view=rev
>> Log:
>> IR: Remove direct comparisons against Metadata::Storage, NFC
>>
>> Modified:
>>     llvm/trunk/lib/IR/Metadata.cpp
>>
>> Modified: llvm/trunk/lib/IR/Metadata.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metada
>> ta.cpp?rev=226490&r1=226489&r2=226490&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/IR/Metadata.cpp (original)
>> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Jan 19 13:26:24 2015
>> @@ -402,7 +402,7 @@ MDNode::MDNode(LLVMContext &Context, uns
>>    for (unsigned I = 0, E = MDs.size(); I != E; ++I)
>>      setOperand(I, MDs[I]);
>>
>> -  if (Storage == Temporary)
>> +  if (isTemporary())
>>      this->Context.makeReplaceable(
>>          make_unique<ReplaceableMetadataImpl>(Context));
>>  }
>> @@ -416,7 +416,7 @@ static bool isOperandUnresolved(Metadata
>>  UniquableMDNode::UniquableMDNode(LLVMContext &C, unsigned ID,
>>                                   StorageType Storage, ArrayRef<Metadata
>> *> Vals)
>>      : MDNode(C, ID, Storage, Vals) {
>> -  if (Storage != Uniqued)
>> +  if (!isUniqued())
>>      return;
>>
>>    // Check whether any operands are unresolved, requiring re-uniquing.
>> @@ -432,7 +432,7 @@ UniquableMDNode::UniquableMDNode(LLVMCon
>>  }
>>
>>  void UniquableMDNode::resolve() {
>> -  assert(Storage == Uniqued && "Expected this to be uniqued");
>> +  assert(isUniqued() && "Expected this to be uniqued");
>>    assert(!isResolved() && "Expected this to be unresolved");
>>
>>    // Move the map, so that this immediately looks resolved.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150120/4a7849ba/attachment.html>