[PATCH] D52907: AMDGPU: Don't merge DS opcodes on SI to fix corruption in Hitman

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In https://reviews.llvm.org/D52907#1260080, @nhaehnle wrote:

> In https://reviews.llvm.org/D52907#1258465, @arsenm wrote:
>
> > In https://reviews.llvm.org/D52907#1258433, @mareko wrote:
> >
> > > In https://reviews.llvm.org/D52907#1258426, @arsenm wrote:
> > >
> > > > Why exactly? Is it possible the Mesa lds allocation isn’t aligning properly?
> > >
> > >
> > > Since the issue only occurs on SI, I don't think Mesa is doing anything bad. Unless there is some LDS hw difference on SI...
> >
> >
> > I do remember one bug we have that may be related. We try to use the ds_read2_b32 with 4-byte signed trick on SI, without checking that we can use the offsets if the base address isn't known positive
>
>
> Are you saying that on SI, if the base address (from VGPR) is negative, but (base address + offset) is in range, the instruction won't execute correctly? Is there documentation on this somewhere?


The problem is specifically on SI the adder for the offset is only 16-bit, so if a carry happens it computes the wrong address. The overly strong condition we use for this is that the base address is known positive (see isDSOffsetLegal)


Repository:
  rL LLVM

https://reviews.llvm.org/D52907