User Details
- User Since
- Oct 29 2014, 9:58 AM (336 w, 3 d)
Fri, Apr 9
Incidentally I think it would be simpler if AMDGPU::getNumFlatOffsetBits did not take a "signed" argument, always returned the total number of bits in the field, and it was the caller's responsibility to worry about whether negative offsets are allowed or not.
Add a DBG_VALUE test case.
Thu, Apr 8
Use eraseFromParentAndMarkDBGValuesForRemoval.
I have no objection to this as a first step in the right direction, but I think it should have no effect at all at the moment. The schedmodel already knows that we can only issue one instruction per cycle, and this patch does not affect that. It only becomes interesting when you model the fact that issuing a trans instruction consumes the trans ALU for (say) 4 cycles, so that on the following 3 cycles you could issue a normal VALU instruction but *not* another trans instruction.
... but adding @rampitec for awareness.
Seems fine to me. There is not much consistency in the naming of the SIInstrFlags flags.
Should I do the same for is_flat_scratch/global?
D76500 has landed now, so maybe abandon this review?
Wed, Apr 7
The '\n' is redundant since MI print includes it already
Tue, Apr 6
I think it was also causing this failure in my Release+Asserts build:
/home/jayfoad2/git/llvm-project/llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll:150:16: error: CHECK-LABEL: expected string not found in input ; CHECK-LABEL: define internal void @branch_funnel(i8* ^ <stdin>:81:16: note: scanning from here define i32 @fn3(i8* nocapture readonly %obj) local_unnamed_addr #2 { ^ <stdin>:95:18: note: possible intended match here define hidden void @__typeid_typeid1_0_branch_funnel(i8* nest %0, ...) local_unnamed_addr #6 { ^ ... Failed Tests (1): LLVM :: Transforms/WholeProgramDevirt/branch-funnel.ll
Thu, Apr 1
Is there some overlap with SIInstrFlags::FLAT, SIInstrFlags::IsFlatScratch, SIInstrFlags::IsFlatGlobal? It seems like your enum is encoding the same thing in a slightly different way. See also the is*FLAT* helper methods in SIInstrInfo.
Rebase.
LGTM.
Looks good to me apart from the G_FCONSTANT issues.
Wed, Mar 31
LGTM, thanks!
Abandoning. It was just an experiment. There is no real use case for it.
Tue, Mar 30
Looks fine to me, thanks.
Seems obviously fine to me.
Please make the commit message a bit more descriptive than just "update".
Looks fine to me modulo the naming nit.
The code change looks fine to me. The added tests aren't actually testing any behaviour that has changed, are they? Is there any reasonable way to add tests for the unable-to-legalize case?
Mon, Mar 29
In general I am uncomfortable about generating code that does not work (i.e. expanding spills the way we do when exec might be 0) and then running yet another pass for correctness to fix it up later. Is there a way this can be made correct by default, and if necessary run an extra pass that optimizes it for efficiency?
Fri, Mar 26
Adding @Joe_Nash since he made some changes related to _e64 suffixes quite recently.