Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp | ||
---|---|---|
394 | switch (NumOp)? | |
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
47 | nit: Bit counting | |
49 | Other *.w nodes are named *_W, so add an underscore for consistency? | |
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td | ||
690–693 | no need to guard with Predicates = [IsLA32]? |
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp | ||
---|---|---|
394 | Thanks, I will change that. | |
llvm/lib/Target/LoongArch/LoongArchISelLowering.h | ||
47 | Thanks, I will change that. | |
49 | Thanks, I will add an underscore. | |
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td | ||
690–693 | It needs to guard with Predicates = [IsLA32], I will fix it, thanks. |
Seems good to me.
P.S. If this lands after D131512 you have to refresh your testcases. But I'd happily rebase if that patch lands after this one too.
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td | ||
---|---|---|
678–685 | Okay so after you grouped the LA32 bit-counting ops together it may be better to move this block below, and add a // Bit counting operations to describe the two groups of operations. Adding these patterns near the multiplications just for saving an if Predicates = [IsLA64] block doesn't help with readability. |
Address @xen0n's comments.
Modify the location of LA64 bit-counting Ops and add comments to increase readability.
nit: Bit counting