Hi Sam, Honestly I don't feel knowledgeable enough to answer this question, so I hope some Turbofan experts can chime in. This line <https://source.chromium.org/chromium/chromium/src/+/main:v8/src/compiler/common-operator.cc;l=1622> defines that the Projection in Turbofan has one value input, a control input and a value output but no control output. I don't understand why it has a control input or what it exactly means that an operation has a control input vs. let's say a Select operation that doesn't have one. Still, I'd assume that the Branch in B4 should have a control input that at some point leads to the same control input as "2: Projection [1]" and therefore I'd assume that it's valid as the control input of the projection dominates the branch that uses it. Looking at the schedule, as the Projection is in B0, it is guaranteed to be "executed" before B4, so the virtual register should be defined and should be "available" / usable by all instructions following itself in B0 and in all instructions in blocks that are reachable from B0 directly or indirectly. I don't think there is anything limiting the liveness of a virtual register, so as long as a block A dominates another Block B, all virtual registers defined in A are usable in B. So your snippet looks totally fine based on my (limited) understanding of Turbofan.
However, I'd assume it should be 1: Projection[0] (0) 2: Projection[1] (0) and not 1: Projection[0] (0) 2: Projection[1] (1) (i.e. both projections should have Int32AddWithOverflow as an input) Best regards, Matthias On Wed, Nov 8, 2023 at 5:24 PM Sam Parker-Haynes <sam.par...@arm.com> wrote: > Hi Matthias, > > My RecreateSchedule example looks like this: > > B0: > 0: Int32AddWithOverflow > 1: Projection[0] (0) > 2: Projection[1] (1) > Branch (2) > ... > > B4: > Branch (2) <-- how should this branch receive (2) > > I know I said it looks okay at the IR level, but since the IR isn't > documented that may not be true ;) So, what are the semantics of a > Projection..? > - Is the Projection[1] of an Overflow operation a part of the control > chain? > - Is it legal for all projected values to be live across basic blocks? If > so, who should be responsible for ensuring values are live where they need > to be? > > Thanks, > Sam > > On Wednesday, November 8, 2023 at 2:15:58 PM UTC Sam Parker-Haynes wrote: > > Also, are any instruction selection tests being added for Turboshaft? > Something like the equivalent of > test/unittests/compiler/arm64/instruction-selector-arm64-unittest.cc? > > On Wednesday, November 8, 2023 at 1:26:12 PM UTC Sam Parker-Haynes wrote: > > EDIT: only passes with --no-turboshaft > > On Wednesday, November 8, 2023 at 1:20:03 PM UTC Sam Parker-Haynes wrote: > > Hi Matthias, > > Ah, I hadn't seen that, so yes I've been tweaking the OG selector function > for arm64. On the x64 side of things, everything is fine with 'pure' > turboshaft pipeline but, as with my arm64 checkout, the issue arises when > modifying the turbofan selector function and using ---no-turboshaft. > > So, I guess the problem is with the RecreateSchedule? > > Thanks, > Sam > > On Wednesday, November 8, 2023 at 1:12:56 PM UTC mlie...@google.com wrote: > > Hi Sam, > > I don't have any insights on x64 for VisitWordCompareZero but as a quick > note, the TurboshaftAdapter instruction selection implementation for > VisitWordCompareZero for arm64 was just added yesterday by me ( > https://chromium-review.googlesource.com/c/v8/v8/+/5002003). > > So, just to clarify, I assume you are talking about the Turbofan {Node* / > TurbofanAdapter} based instruction selection with Turboshaft optimizations > followed by a RecreateSchedule? > If you do changes for the TurbofanAdapter instruction selection on arm64 > for code that already has an implementation for the TurboshaftAdapter, > could you add a TODO(mliedtke) on the Turboshaft implementation with some > nice description and CC me on the gerrit changes, so these don't get lost > when we switch over to the TurboshaftAdapter? > > Thanks and best regards, > Matthias > > On Wed, Nov 8, 2023 at 1:54 PM Sam Parker-Haynes <sam.p...@arm.com> wrote: > > Hi, > > I've been investigating rematerialising flag setting instructions in > VisitWordCompareZero, just by removing the CanCover check, and something is > going wrong with turboshaft. > > I have an arithmetic overflow operation, Int32AddWithOverflow, and the > overflow check is used by two branch operations. With a turbofan-only flow, > the overflow operation and the projection will be duplicated for the second > branch, and everything is fine. But with turboshaft, neither the overflow > operation or projection are duplicated and the second branch uses the > original projection[1] value. This looks fine at the IR level, but ends up > broken after isel when the register allocator comes across a virtual > register without a definition. This happens for both x64 and arm64, so I'm > assuming turboshaft is making some assumptions, based on the current > behaviour, that are non-obvious to me. > > Any ideas? > > Thanks! > Sam > > > > -- > -- > v8-dev mailing list > v8-...@googlegroups.com > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to v8-dev+un...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-dev/175a4ba2-9d33-45bb-84e3-91d41a722846n%40googlegroups.com > <https://groups.google.com/d/msgid/v8-dev/175a4ba2-9d33-45bb-84e3-91d41a722846n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > -- > -- > v8-dev mailing list > v8-dev@googlegroups.com > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to v8-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-dev/46693e6c-1216-493b-a104-2c9d41075987n%40googlegroups.com > <https://groups.google.com/d/msgid/v8-dev/46693e6c-1216-493b-a104-2c9d41075987n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CACx9iAPRcjMV_H2Lw1ZR2uVvDWpqat%3DsWHKuHCdKQC7pThwBFA%40mail.gmail.com.
