Depends on D128898
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td | ||
---|---|---|
17–20 | Actually I don't quite get why this needs to be different just for LA64. Isn't the behavior for these two insns identical between LA32 and LA64? | |
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp | ||
140–142 | ||
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
38 | movgr2fr is definitely not "FP to int". Maybe this comment should be fixed. |
llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td | ||
---|---|---|
17–20 | Yes, but we only use these two SDNodes for LA64. In LA32, we use bitconvert, such as (LoongArchFloat32InstrInfo. td, line 224-227) | |
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp | ||
140–142 | Thanks for the suggestion, I will change that. | |
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
38 | Thanks. I will change that. |
Still not quite sure about the necessity of the _LA64 nodes/pats (is it again because of tblgen intricacies?) but the rest looks okay.
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
---|---|---|
42 | I'm not quite sure about what the "LA" suffix means. Aren't we obviously LoongArch? Or is this meaning "L(64-bit integer), Any FP precision"? This seems likely until I find both "w" and "l" ftintrz's are pattern-matched from this. |
Yes, we want to reduce the intricacies of tblgen and use the name to identify this node only for LA64.
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
---|---|---|
42 | I had hoped to use "LA" for "LoongArch". Thanks for your reminding, I found this suffix redundant for the FTINT node, I will remove it. |
I don't know if the LA64-specific DAG nodes could actually be removed for further simplicity, but the rest seems good to me. Someone could enlighten me on this.
llvm/lib/Target/LoongArch/LoongArchFloat32InstrInfo.td | ||
---|---|---|
25 | What does anyext mean? Either this isn't meaningful and should be removed, or an explanatory comment should be around. | |
llvm/lib/Target/LoongArch/LoongArchFloat64InstrInfo.td | ||
201 | nit: "FP" or "Floating-point" for consistency. |
Actually I don't quite get why this needs to be different just for LA64. Isn't the behavior for these two insns identical between LA32 and LA64?