[PATCH] D64966: [AMDGPU/GlobalISel] Add llvm.amdgcn.fdiv.fast legalization.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:659
+                                                     bool HasSideEffects,
+                                                     Optional<unsigned> Flags) {
   auto MIB =
----------------
Should default to None.

I'm not sure if this one should get a flags argument. You can set it on the instruction separately from constructing it


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:676
+                                                     bool HasSideEffects,
+                                                     Optional<unsigned> Flags) {
   auto MIB =
----------------
Should default to None


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:711
+                                                const SrcOp &Op1,
+                                                Optional<unsigned> Flags) {
 
----------------
Should default to None


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:720
+                                                  const SrcOp &Op1,
+                                                  Optional<unsigned> Flags) {
 
----------------
Should default to None


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-intrinsic-amdgcn-fdiv-fast.mir:28
+    $vgpr0 = COPY %2
+...
----------------
Should add another test with the flags


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64966/new/

https://reviews.llvm.org/D64966