Jump to conversation
Unresolved conversations (1)
@sam-mccall sam-mccall Sep 26, 2023
this sort() seems to have introduced nondeterminism in some configs I've replaced it with stable_sort in 679c3a1791b51f2ad0771b8ce924130b071a956f - maybe you have a better fix, but this seemed like a safe way to avoid reverting. (My reading is the old code picks the *first* best option & stable_sort restores this)
...b/CodeGen/SelectionDAG/TargetLowering.cpp
nickdesaulniers
Resolved conversations (7)
@qcolombet qcolombet Sep 22, 2023
I find it strange to have `true` when the input is not an immediate. I guess the rationale hinge on the `IfPossible` part of the function name. How does the code look like if you were to return a `std::optional<bool>` and return `none` for no immediate?
...b/CodeGen/SelectionDAG/TargetLowering.cpp
nickdesaulniers
@qcolombet qcolombet Sep 22, 2023
```suggestion for (unsigned end = G.size(); BestIdx != end && !lowerImmediateIfPossible(G[BestIdx], Op, DAG, *this), ++BestIdx) {} if (BestIdx == end) return; ```
Outdated
...b/CodeGen/SelectionDAG/TargetLowering.cpp
nickdesaulniers
@qcolombet qcolombet Sep 22, 2023
I'm nervous when I see loops without clear bound. Why don't we have to check that `BestIdx < G.size()` to prevent out-of-bounds accesses?
Outdated
.../CodeGen/GlobalISel/InlineAsmLowering.cpp
nickdesaulniers
@bwendling bwendling Sep 21, 2023
I don't quite get what the numbers represent here. Is "higher" more preferable or is "lower"? If lower is preferable, then shouldn't `C_Immediate` be more like `C_Register` rather than `C_Other`? Also, please reorder the cases into the sort order...
...b/CodeGen/SelectionDAG/TargetLowering.cpp
nickdesaulniers
@bwendling bwendling Sep 21, 2023
s/contains/contain/
Outdated
llvm/include/llvm/CodeGen/TargetLowering.h
nickdesaulniers
@bwendling bwendling Sep 21, 2023
Is the `const` necessary here?
Outdated
llvm/include/llvm/CodeGen/TargetLowering.h
nickdesaulniers
@bwendling bwendling Sep 21, 2023
Is the `const` required here?
Outdated
llvm/lib/Target/AMDGPU/SIISelLowering.h
nickdesaulniers