• Rename argument from $ty to $arg

• Address comments

While it would be nice for dereferenceable to not imply nonnull, the implementation currently assumes it does and will speculate loads based on that. See llvm::Value::getPointerDereferenceableBytes and its users.

Let's land this now to unblock asan. It's breaking test/Transforms/test-legalizer.mlir though because there's a cast being left behind.

looks good

In D17993#2409401, @brooksmoses wrote:

So, I have bad news: This causes OpenJDK to segfault. The relevant code is here:
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/arena.cpp#L311

void Arena::destruct_contents() {
  if (UseMallocOnly && _first != NULL) {
    char* end = _first->next() ? _first->top() : _hwm;
    free_malloced_objects(_first, _first->bottom(), end, _hwm);
  }
  // reset size before chop to avoid a rare racing condition
  // that can have total arena memory exceed total chunk memory
  set_size_in_bytes(0);
  _first->chop();
  reset();
}

I've also seen a segfault in Verilator that root-causes to this patch, though I haven't yet tracked that down to the source code.

I hate to say it, but is this a significant enough problem to call for a (temporary, I hope) rollback?

Saw some random miscompiles after this. Fixed by 4dbe12e86649ba6b5f03a9ba97e84d718727f7a7, can you check if I got it right?

looks good

Can you add a note to the commit description that this is a reland of D90379?

In D90379#2370654, @frgossen wrote:

Yes, I reverted it yesterday as it caused problems with the integrate. I will re-land this when I figure out how to install an old GCC version.

Building with GCC7 is fixed by 5ece2115d85326e3b24336a7ccb4c191932ccb4a, but looks like this was reverted already.

Should be fixed as of 5ece2115d85326e3b24336a7ccb4c191932ccb4a

looks good

Thanks

thanks

I have no memory of adding that transformation 9 years ago. dec + test should be better than popcnt + cmp even on modern x86, but with no data to back that up it might not matter at all.

This should be safe. Looks good.

The code now essentially says

looking good

• Don't print "std." prefix on tranpose

• Address a round of review comments

looks good

Thanks

Can we promote everything to 64 bits to simplify the logic a bit? Otherwise looking good.

Reproduces for me. Let's roll this back for now.

Ok, let's revert this for now. It's annoying to have a dependency on something that's not used by the in-tree backends, so we need to figure out how to remove it.

looks good

Thanks, I prefer simplicity over efficiency here.