Page MenuHomePhabricator

[DSE] Queue non-memory dead instructions for later removal.
Needs ReviewPublic

Authored by fhahn on Mar 9 2021, 12:52 PM.

Details

Summary

This patch updates DSE's removal strategy to initially only remove the
memory instruction and queue now-dead operands for later removal.

This is required to enable PHI translation while looking for candidates
for removal, because the translated result may reference IR instructions
that would get removed before being used.

Diff Detail

Event Timeline

fhahn created this revision.Mar 9 2021, 12:52 PM
fhahn requested review of this revision.Mar 9 2021, 12:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2021, 12:52 PM

I think, more robust approach would be not to depend on existing instructions but create the required ones during phi translation as needed. Phi translation already does this to some extent. In D90095 (which prototypes PHI translation support in DSE) pointers are simplified by GetPointerBaseWithConstantOffset during PHI translation what effectively removes dependencies on existing GEPs and BitCasts.

What is your motivating example?

fhahn added a comment.Jun 28 2021, 9:17 AM

I think, more robust approach would be not to depend on existing instructions but create the required ones during phi translation as needed. Phi translation already does this to some extent. In D90095 (which prototypes PHI translation support in DSE) pointers are simplified by GetPointerBaseWithConstantOffset during PHI translation what effectively removes dependencies on existing GEPs and BitCasts.

What is your motivating example?

I think even with GetPointerBaseWithConstantOffset, we could still rely on existing GEPs, e.g. if one of the indices is a non-constant value, so IIUC we could still run into problems when removing such GEPs as we go along?

An example with a crash was shared in D98288. IIUC this could be modified to use an argument value as index in the GEPs to also cause a crash with the GetPointerBaseWithConstantOffset approach.

%"struct.type" = type <{ i8 }>

; Function Attrs: noreturn
define void @f() {
entry:
  br label %while.cond

while.cond:                                       ; preds = %exit, %entry
  br i1 undef, label %if.end.i, label %if.then.i

if.then.i:                                        ; preds = %while.cond
  br label %exit

if.end.i:                                         ; preds = %while.cond
  store i8* undef, i8** inttoptr (i64 16 to i8**), align 16
  %.cast1.i = bitcast i8* undef to %"struct.type"*
  %IsMemberPointer.i.i = getelementptr inbounds %"struct.type", %"struct.type"* %.cast1.i, i64 0, i32 0
  store i8 0, i8* %IsMemberPointer.i.i, align 4
  br label %exit

exit: ; preds = %if.end.i, %if.then.i
  %retval.0.i = phi %"struct.type"* [ undef, %if.then.i ], [ %.cast1.i, %if.end.i ]
  %IsMemberPointer = getelementptr inbounds %"struct.type", %"struct.type"* %retval.0.i, i64 0, i32 0
  store i8 1, i8* %IsMemberPointer, align 4
  br label %while.cond
}