This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Add user interface for DetectDeadLanes
AbandonedPublic

Authored by BeMg on Dec 20 2022, 2:06 AM.

Details

Summary

Let other class could reuse the used/def of subregister from DetectDeadLanes pass.

Diff Detail

Event Timeline

BeMg created this revision.Dec 20 2022, 2:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 20 2022, 2:06 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
BeMg requested review of this revision.Dec 20 2022, 2:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 20 2022, 2:06 AM
craig.topper added inline comments.Dec 21 2022, 10:33 AM
llvm/lib/CodeGen/DetectDeadLanes.cpp
502

Do we need to skip the "Mark operands as dead/unused." part of runOnce for this usage?

arsenm added inline comments.Dec 21 2022, 10:36 AM
llvm/include/llvm/CodeGen/DetectDeadLanes.h
106–107

Keep member variables together below here

llvm/lib/CodeGen/DetectDeadLanes.cpp
486–507

Why is this all repeated here?

BeMg updated this revision to Diff 484752.Dec 22 2022, 12:56 AM
  1. let runOnMachineFunction reuse getSubRegisterLaneBitInfo
  2. Move the variable
BeMg marked 2 inline comments as done.Dec 27 2022, 5:44 AM
BeMg added inline comments.
llvm/lib/CodeGen/DetectDeadLanes.cpp
502

I'm not sure whether "Mark operands as dead" is part of computing DeadLanes algorithm. And we remove "Mark operands as dead" may get an unaccomplished result?

BeMg updated this revision to Diff 485948.Jan 3 2023, 2:52 AM

rebase

BeMg updated this revision to Diff 486463.Jan 4 2023, 8:53 PM

Add a parameter to control whether set Operand as undef/def

craig.topper added inline comments.Jan 4 2023, 9:19 PM
llvm/lib/CodeGen/DetectDeadLanes.cpp
443

Can we split this function into two functions above and below this point and check this flag in the while loop?

486–487

This debug message doesn't make sense for the call from the RISC-V pass.

518

This comment belongs to the if (!MRI->subRegLivenessEnabled()) that got moved.

craig.topper added inline comments.Jan 4 2023, 9:21 PM
llvm/lib/CodeGen/DetectDeadLanes.cpp
443

let me rephrase

Can we split this function into two functions above and below this point? Have the while loop in the caller only call the second half when NeedSetUndef is true. The first function can just return void since it doesn't modify anything.

BeMg updated this revision to Diff 486477.Jan 4 2023, 11:36 PM

Split runOnce function into ModifySubRegisterOperandStatus and computeSubRegisterLaneBitInfo

craig.topper added inline comments.Jan 5 2023, 11:30 AM
llvm/lib/CodeGen/DetectDeadLanes.cpp
440

Function names should be start with a lower case letter

505

We don't need to call computeSubRegisterLaneBitInfo if ModifySubRegisterOperandStatus didn't change anything

arsenm added inline comments.Jan 5 2023, 11:32 AM
llvm/include/llvm/CodeGen/DetectDeadLanes.h
97

Needs comment, I don't know what the two return values mean.

llvm/lib/CodeGen/DetectDeadLanes.cpp
473

I don't really understand what again is supposed to mean (but I guess this was the case before too)

BeMg updated this revision to Diff 487359.Jan 9 2023, 2:53 AM
  1. ModifySubRegisterOperandStatus -> modifySubRegisterOperandStatus
  2. if not change anything, skip computeSubRegisterLaneBitInfo
  3. Add comment
craig.topper added inline comments.Jan 9 2023, 12:19 PM
llvm/lib/CodeGen/DetectDeadLanes.cpp
497

Change the loop back to do while and just break in the middle like:

do {
  computeSubRegisterLaneBitInfo();
  if (!NeedSetUndef)
    break;

  bool LocalChanged;
  std::tie(LocalChanged, Again) = runOnce(MF);
  Changed |= LocalChanged;
} while(Again);
BeMg updated this revision to Diff 487662.Jan 9 2023, 8:42 PM

Use do while

craig.topper added inline comments.Jan 17 2023, 9:28 PM
llvm/include/llvm/CodeGen/DetectDeadLanes.h
41

We shouldn't have "using namespace llvm" in a header file

BeMg abandoned this revision.Feb 20 2023, 9:47 PM

D141993 solved it.