Page MenuHomePhabricator

Add type attributes to LLVM C API
ClosedPublic

Authored by lerno on Mar 2 2021, 4:51 AM.

Details

Summary

The LLVM C API is missing type attributes as is needed by attributes such as sret and byval. This patch adds three missing wrapper functions.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=48249

Diff Detail

Unit TestsFailed

TimeTest
50 msx64 debian > Flang.Semantics::resolve102.f90
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/test_errors.sh /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/resolve102.f90 /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/test/Semantics/Output/resolve102.f90.tmp /mnt/disks/ssd0/agent/llvm-project/build/bin/f18 -intrinsic-module-directory /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/include/flang

Event Timeline

lerno created this revision.Mar 2 2021, 4:51 AM
lerno requested review of this revision.Mar 2 2021, 4:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2021, 4:51 AM
arsenm added a comment.Mar 2 2021, 5:18 AM

As far as I know we don't have any C API unit tests, but sorely need them if you want to try adding the first

lerno added a comment.Mar 2 2021, 5:25 AM

As far as I know we don't have any C API unit tests, but sorely need them if you want to try adding the first

I see echo.cpp utilizing LLVMCreateEnumAttribute (but nothing using LLVMCreateStringAttribute), but I am not familiar with that code.

I submitted this change as there seemed little interest in https://bugs.llvm.org/show_bug.cgi?id=48249 from anyone else and there was a suggestion on the Discord that if I submitted a patch then maybe someone could have a look at it. The code is straightforward, but I don't know LLVM well enough to set up any tests. Both my project (C3) and the Odin Language is looking to use these functions as soon as they are included.

lerno added a comment.Mar 2 2021, 7:31 AM

Just so I understand @arsenm: are API unit tests needed to approve this pull request? If so, who is planning on adding them? I mean the C API as it currently works cannot even correctly provide byval and sret according to spec.

arsenm accepted this revision.Mar 3 2021, 6:16 AM

Just so I understand @arsenm: are API unit tests needed to approve this pull request? If so, who is planning on adding them? I mean the C API as it currently works cannot even correctly provide byval and sret according to spec.

No, but the C API is likely to keep breaking unless somebody adds them

This revision is now accepted and ready to land.Mar 3 2021, 6:16 AM