This patch fixes calculating address of arguments larger than 32 bit on stack for variadic functions (rounding up address to alignment) on ppc32 architecture.
Details
Diff Detail
Event Timeline
lib/CodeGen/TargetInfo.cpp | ||
---|---|---|
3549 | You need to use variable names consistent with our coding convention (OverflowArgArea, etc.). |
lib/CodeGen/TargetInfo.cpp | ||
---|---|---|
3550 | This patch applies to more types than just double. You should probably add tests for some of the other cases, but if not, please at least make your commit message convey the intended generality. | |
3551 | if (Align > OverflowAreaAlign) | |
3561 | Would you mind extracting a function to do this realignment? There are 3-4 other places in this file that do exactly this. Using the existing code pattern from emitVoidPtrDirectVAArg — i.e. using an add instead of a non-inbounds GEP — seems reasonable. I think this is a sensible prototype: static Address emitRoundPointerUpToAlignment(CodeGenFunction &CGF, llvm::Value *Ptr, CharUnits Align, const llvm::Twine &Name); | |
3564 | This code will be more readable if you keep Align as a CharUnits. |
lib/CodeGen/TargetInfo.cpp | ||
---|---|---|
252 | Thank you for extracting this. First, this function deserves a doc comment now; I would suggest: /// Dynamically round a pointer up to a multiple of the given alignment. Also, this is now generically useful, so (1) please rename OverflowArgArea to something more generic, like Ptr, and (2) please use LLVM value names based on the name already in Ptr. That is, instead of "overflow_arg_area.align", please use Ptr->getName () + ".aligned" Finally, please look for the other places in this file that you could change to use this new function. There's one in emitVoidPtrDirectVAArg, and there are several other in other targets. | |
3553 | Please sink OverflowArgArea into the if block, since you don't need it outside. |
Please feel free to commit after you make this last change.
lib/CodeGen/TargetInfo.cpp | ||
---|---|---|
216 | LLVM code style is to put the else on the same line as the closing brace. |
Minor code-style nitpick: please put the * next to the function name, not the type.