[PATCH] D38607: [AMDGPU] New 64 bit div/rem expansion

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:1623
+
+    // if C3 != 0 ...
+    SDValue Sub2_Lo = DAG.getNode(ISD::SUBCARRY, DL, HalfCarryVT, Sub1_Lo,
----------------
rampitec wrote:
> b-sumner wrote:
> > It wouldn't hurt to have more comments expressing the algorithm in a C-like style.
> This is more like todo: we can insert this "if" here and split the BB. That is questionable though if it is profitable in the divergent control flow and not really easy during the lowering. It is substituted by the cndmask after endif comments. Anyway, comment is left if later we decide to use if/then here.
This is why D18072 was going to be an IR expansion of this. The comment should be more descriptive that it may be beneficial to expand this with control flow outside of the DAG to allow skipping over this portion.


https://reviews.llvm.org/D38607