Page MenuHomePhabricator

AMDGPU: Fix memory dependence analysis by considering the offset.

Authored by cfang on Feb 15 2019, 11:09 AM.



In SIInstrInfo::areMemAccessesTriviallyDisjoint, the current implementation ignored the offset when creates MemoryLocation
for alias analysis, and thus may report wrong result when offset is not zero. This patch fixes the issue.

Diff Detail


Event Timeline

cfang created this revision.Feb 15 2019, 11:09 AM
arsenm requested changes to this revision.Feb 15 2019, 11:16 AM
arsenm added inline comments.
2218–2232 ↗(On Diff #187050)

Why can't you use MI.mayAlias and delete this?

This revision now requires changes to proceed.Feb 15 2019, 11:16 AM
cfang marked an inline comment as done.Feb 15 2019, 1:54 PM
cfang added inline comments.
2218–2232 ↗(On Diff #187050)

This intends to fix a bug in an application. We can use MI.mayAlias and delete areMemAccessesTriviallyDisjoint.
However, we need to make sure areMemAccessesTriviallyDisjoint is indeed useless because things developed there
are not for on reason. We need extensive tests.
So I propose to add this patch first to fix the bug. Then we can spend time to investigate the way to safely remove the stuff in
areMemAccessesTriviallyDisjoint and replace it with MI.mayAlias.

cfang updated this revision to Diff 187272.Feb 18 2019, 2:09 PM

Use MachineInstr::mayAlias to replace areMemAccessesTriviallyDisjoint in LoadStoreOptimizer pass.

cfang updated this revision to Diff 187273.Feb 18 2019, 2:32 PM

Remove the unused Target Instruction Info argument in a couple functions.

arsenm accepted this revision.Feb 18 2019, 2:33 PM


69 ↗(On Diff #187272)

Typo oroginal

This revision is now accepted and ready to land.Feb 18 2019, 2:33 PM
arsenm added inline comments.Feb 18 2019, 2:34 PM
264–265 ↗(On Diff #187273)

This merged part of the other patch

cfang marked an inline comment as done.Feb 18 2019, 2:44 PM
cfang added inline comments.
264–265 ↗(On Diff #187273)

What do you mean here? I think this is the staring point of the current patch. We are going to replace
TII->areMemAccessesTriviallyDisjoint with MI.mayAlias to fix the bug.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2019, 3:01 PM