[PATCH] D140530: [RISCV] Add integer scalar instructions to isAssociativeAndCommutative

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:2616-2618
+  // If both instructions have no FRM operand, treat it as equal FRM.
+  if (MI1FrmOpIdx < 0 && MI2FrmOpIdx < 0)
+    return true;
----------------
HsiangKai wrote:
> kito-cheng wrote:
> > This seems out of scope of this patch?
> In order to let hasReassociableSibling() return correct result for integer scalar cases, we have to handle non-fp cases here.
> 
> I think it makes sense to return true when instructions have no FRM operand in this function. There may be better way to handle it for hasReassociableSibling(). Any suggestions?
Thanks for the explanation, the function name let me feel that's unrelated to integer operations, however I think this kind of change the behavior of this function, next if-condition is 
```
  if (MI1FrmOpIdx < 0 || MI2FrmOpIdx < 0)
    return false;
```

I didn't have idea how to improve that yet, but I feel it should be add check logic in `hasReassociableSibling` rather than here.

Anyway I don't have strong objection here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140530