[PATCH] D106447: [DAGCombine] DAGTypeLegalizer::GenWidenVectorLoads(): make use of dereferenceability knowledge

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added a comment.

In D106447#2899456 <https://reviews.llvm.org/D106447#2899456>, @lebedev.ri wrote:

> In D106447#2899434 <https://reviews.llvm.org/D106447#2899434>, @sebastian-ne wrote:
>
>> There were some concerns that assuming that dereferenceable bytes >= alignment is incorrect. Memory protection may work at 4-byte or even byte granularity and objects may be over-aligned in memory. In these cases, loading padding bytes does not work.
>>
>> Even if it can be proven that enough bytes are dereferenceable,
>
> This is FUD.
>
> …
>
> We can not second-guess LLVM semantics.
> If we can tell that we are always allowed to load N bytes without trapping, then we can do that.
> If we actually can't, then the problem is elsewhere, we shouldn't have deduced that we can.

I agree with that, I misunderstand the code then.

  if (LD->isSimple()) {
    NumDereferenceableBytes = LD->getAlignment();
    if (!LdWidth.isScalable())
      NumDereferenceableBytes =
          std::max<unsigned>(NumDereferenceableBytes, LdWidth / 8);
    if (!WidenWidth.isScalable() && NumDereferenceableBytes < WidenWidth / 8 &&
        LD->getPointerInfo().isDereferenceable(
            WidenWidth / 8, *DAG.getContext(), DAG.getDataLayout()))
      NumDereferenceableBytes =
          std::max<unsigned>(NumDereferenceableBytes, WidenWidth / 8);
  }

As far as I understand this part, `NumDereferenceableBytes` is set to the (guaranteed minimum) alignment of the load.
In some cases, where it’s ok, e.g. because the dereferenceable size is set, `NumDereferenceableBytes` is increased.

So, `NumDereferenceableBytes` is always at least the alignment?
`isDereferenceable(LD->getAlignment())` is never checked as far as I see.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106447/new/

https://reviews.llvm.org/D106447