Register Calling Convention defines a new behavior for v64i1 types.
This type should be saved in GPR.
However for 32 bit machine we need to split the value into 2 GPRs (because each is 32 bit).
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Hi, please change
Variable names should be camel case, and start with an upper case letter (e.g. Leader or Boats).
lib/Target/X86/X86CallingConv.h | ||
---|---|---|
14 | This function is too big to be inline. Can you move it to .c** file? | |
21 | dot at the end of the sentence. | |
22 | What is this list for? | |
26 | std::vector is not "popular" in the LLVM code. SmallVector can be used instead. | |
37 | Why 2 ? Please add comments here. | |
40 | Does this loop allays have 2 iterations? |
lib/Target/X86/X86CallingConv.h | ||
---|---|---|
22 | This is the list of registers that can be used to pass the value, according to regcall calling convention. |
lib/Target/X86/X86CallingConv.cpp | ||
---|---|---|
26 | #defines like this are not allowed in LLVM, I think. | |
43 | How the case is handled when there is no available registers? I did not see assign-to stack for this type. | |
49 | All variables names should start with capital letter | |
lib/Target/X86/X86CallingConv.td | ||
91 | do you implement CC_X86_RegCall_Error for the types that will come in the next commit? | |
lib/Target/X86/X86ISelLowering.cpp | ||
2372 | Please add 2 words about the Root, VA and NextVA arguments in doxygen format. And InFlag role is not clear. | |
2376 | BMI implies BWI | |
2410 | you can combine this "if" with the previous one, right? | |
2431 | do not pass calling convention here | |
2438 | compilation will fail for unreferenced variable in the buildbot. | |
2445 | unreachable or assert - ? | |
2449 | The function may return SDValue. But you do many asserts and one bitcast. Do you really need a function? | |
2495 | wrong line alignment, function name lower case | |
2515 | the original condition for this operation was VA.getValVT().getScalarType() == MVT::i1 | |
2644 | Why do we need this additional condition? Please add comments. | |
2769 | wrong line alignment | |
2882 | I don't understand this change | |
3317 | Why do you drop SIGN_EXTEND? |
lib/Target/X86/X86CallingConv.cpp | ||
---|---|---|
43 | Since v64i1 is promoted to i64, the following rule will assign to stack (in case we don't have available registers): // In 64 bit, assign 64/32 bit values to 8 byte stack CCIfSubtarget<"is64Bit()", CCIfType<[i32, i64, f32, f64], CCAssignToStack<8, 8>>>, | |
lib/Target/X86/X86CallingConv.td | ||
91 | Yes | |
lib/Target/X86/X86ISelLowering.cpp | ||
2410 | I can do so but it will require duplication of the following lines: // Convert the i32 type into v32i1 type Lo = DAG.getBitcast(MVT::v32i1, ArgValue); ... // Convert the i32 type into v32i1 type Hi = DAG.getBitcast(MVT::v32i1, ArgValue); So i prefered to seprate it into two different if statements. | |
2438 | It is referenced in the next line. It will be referenced in the future as well. | |
2445 | This decision was made due to future design. | |
2495 | The alignment was created by clang-format. | |
2769 | Clang-format did it | |
2882 | LLVM Coding Standards prefer all variables to be capitalized including iterator. for (unsigned I = 0, E = List.size(); I != E; ++I) if (List[I]->isFoo()) return true; | |
3317 | It resides inside the function |
lib/Target/X86/X86CallingConv.td | ||
---|---|---|
91 | I think we can stay with TODO comment till the next commit and remove CC_X86_RegCall_Error |
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
2410 | Use ArgValueLo and ArgValueHi inside if-else blocks |
lib/Target/X86/X86CallingConv.td | ||
---|---|---|
91 | The custom function was introduced in the previous commit and was requested by other reviewers. |
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
2411 | Please refer to my comment here |
This function is too big to be inline. Can you move it to .c** file?