This is the target-specific part of http://reviews.llvm.org/D15340
Details
Diff Detail
Event Timeline
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
10055 | Do you need to emit any kind of CFI pseudo-instructions for these so that unwinding works? |
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
10055 | This is a good point. I think we should but I am not so sure. Maybe Nick is a better person to answer the question on Darwin platform. Specific to C++-style TLS access functions, they should be nounwind. Throwing an exception from the initializer of a thread-local variable is defined to immediately result in a call to std::terminate(); there’s no legal way for unwinding to actually leave such a function. (quote from John McCall) :] |
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
10055 | In that case, please add a comment about this. If someone goes to generalize this later (or port it to another target), it should be obvious that this is something they'll need to think about. |
Addressing Hal's comment. Patch is also updated to work with the target independent portion (http://reviews.llvm.org/D15340).
Hi Manman,
Thanks for your patience.
Here are a few comments.
Cheers,
-Quentin
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
3276 | Why is this using a pointer type instead of an i64? | |
3277 | No need for {} | |
3280 | Ditto. | |
3283 | Was this clang-formated? | |
10059 | Could you assert that the MachineFunction have the nounwind attribute? | |
lib/Target/AArch64/AArch64ISelLowering.h | ||
392 | I think we should change the hook to take a MachineFunction. | |
lib/Target/AArch64/AArch64MachineFunctionInfo.h | ||
75 | Add a comment for that field. | |
lib/Target/AArch64/AArch64RegisterInfo.cpp | ||
61 | Isn’t this redundant with the isSplitCSR check? | |
test/CodeGen/AArch64/cxx-tlscc.ll | ||
50 | Nice! :) |
Thanks Quentin for reviewing the patch!
Addressing his comments with the updated patch.
Addressing review comments,
Cheers,
Manman
lib/Target/AArch64/AArch64RegisterInfo.cpp | ||
---|---|---|
61 | If we are going to support other calling conventions with split CSR, it will be listed here with "else if", so we know which corresponding ViaCopy_SaveList to use. Similarly in getCalleeSavedRegs above. |
I think we should change the hook to take a MachineFunction.
That way we could check that the function does have the nounwind attribute and if it does not, we can say this is not supported.