support more names from MS #1338
Conversation
|
@wsmoses This pr allows enzyme to work better with MS's ABI. Please let me know anything I can do further to get this merged. I am not sure how the testing system expects to process the attached ll file. |
|
@wsmoses If this last commit is not the way to run the integration, please let me know. This PR should be close to being over the line. |
|
@wsmoses I see that the CI for LLVM 15 has failed with the error message |
|
Yeah if you look at other test files, there should be some CHECK and
CHECK-NEXT lines at the bottom that will test if the output of the AD
matches some desired results.
That’s missing here hence the error
…On Wed, Aug 16, 2023 at 9:33 AM dan ***@***.***> wrote:
@wsmoses <https://github.com/wsmoses> I see that the CI for LLVM 15 has
failed with the error message
error: no check strings found with prefix 'CHECK:'
Please advise on what to put after these CHECKs. I do not understand the
existing examples.
—
Reply to this email directly, view it on GitHub
<#1338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJTUXBXYVAEBUWGIVKXRNTXVQIMHANCNFSM6AAAAAA2PAFIIA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I’d personally run the command on the file to find the output, check that
it it is as expected, then use that to write the run line
…On Wed, Aug 16, 2023 at 9:44 AM Billy Moses ***@***.***> wrote:
Yeah if you look at other test files, there should be some CHECK and
CHECK-NEXT lines at the bottom that will test if the output of the AD
matches some desired results.
That’s missing here hence the error
On Wed, Aug 16, 2023 at 9:33 AM dan ***@***.***> wrote:
> @wsmoses <https://github.com/wsmoses> I see that the CI for LLVM 15 has
> failed with the error message
> error: no check strings found with prefix 'CHECK:'
> Please advise on what to put after these CHECKs. I do not understand the
> existing examples.
>
> —
> Reply to this email directly, view it on GitHub
> <#1338 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJTUXBXYVAEBUWGIVKXRNTXVQIMHANCNFSM6AAAAAA2PAFIIA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
|
The output file has about 400 lines. From the other examples I saw, it looks like not all lines need to be included. Is this correct, that is, can I just select a few choice lines and include those in the check? |
| } else if (auto arg = dyn_cast<GlobalVariable>(oval)) { | ||
| if (arg->getName() == "_ZTVN10__cxxabiv120__si_class_type_infoE" || | ||
| arg->getName() == "_ZTVN10__cxxabiv117__class_type_infoE" || | ||
| arg->getName() == "_ZTVN10__cxxabiv121__vmi_class_type_infoE") | ||
| arg->getName() == "_ZTVN10__cxxabiv121__vmi_class_type_infoE" || | ||
| arg->getName().startswith("??_R")) // any of the MS RTTI manglings |
There was a problem hiding this comment.
Should all MS mangled globals have this treatment though? What about user defined globals!
There was a problem hiding this comment.
I didn't like this either but there were too many, and I didn't know how many more there could be. I think this is not a problem because the closest an MS user can get from C/C++ is ?_R... which has one question mark and comes from _R. Besides, such Identifiers are reserved and shouldn't be used anyways.
| !17 = !{!18, !10, i64 8} | ||
| !18 = !{!"?AUObject@@", !10, i64 8} | ||
|
|
||
| ; CHECK: %rtti.CompleteObjectLocator = type { i32, i32, i32, i32, i32, i32 } |
There was a problem hiding this comment.
I would change the check lines to be the generated derivative functions
|
@wsmoses this broke LLVM-16 CI https://github.com/EnzymeAD/Enzyme/actions/runs/6043182139/job/16399729651?pr=1397 |
How do you feel about in Enzyme.cpp replacing all of the
metaString->startswith("enzyme_")by
metaString->contains("enzyme_")?
These can be mangled too.