User Details
- User Since
- Oct 4 2018, 11:50 PM (195 w, 11 h)
Wed, Jun 29
Mon, Jun 6
Thu, Jun 2
LGTM. But add the reference (https://reviews.llvm.org/D126838) to the commit message to indicate which 'earlier change'. Also let us merge it unti kernel side change is in reasonable shape.
May 24 2022
@compnerd provided a better fix https://reviews.llvm.org/D126093, so I am going to close this one.
Sorry. I missed this patch somehow and did another patch https://reviews.llvm.org/D126223. My patch skipped TL.setNameLoc(DS.getTypeSpecTypeNameLoc()); which probably not correct. Accept this patch and abandon my patch D126223.
May 23 2022
May 20 2022
@compnerd Thanks for reporting. I will take a look as soon as possible.
May 12 2022
- also set signed enum for dwarf::DW_ATE_signed_char int.
May 11 2022
- change kflag meaning: kflag = 0 for unsigned and kflag = 1 for signed.
- change the ordering of enum64 val_lo32 and val_hi32.
May 10 2022
@Jim Thanks for the updated commit message. Please go ahead to merge to llvm-project main branch if you have write permission. Otherwise, please provide me with peoper git author signature "<name> <email address>" and I can help push the patch for you.
Could you provide proper "<Name> <Email Address>" so I can change author to you when I push the commit to the main branch?
The current 'summary' or commit message is too simple. Could you clarify some motivation why you need to add this instruction?
The change itself looks good to me. The kernel selftest test_verifier does test BPF_MOD instruction and I checked verifier code, BPF_MOD is handled similar to BPF_DIV which should be okay in most cases, so I didn't see a major issue here.
The change looks good to me. Do you have write permission to merge into llvm-project main repo? If not, I can do the merge for you.
May 4 2022
please having proper reviewers. Me, @anakryiko @ast are not the right persons to review this patch.
Apr 28 2022
Apr 27 2022
The BPF doesn't really recommend to compile from the assembly code to the object code, so we should be fine here. But there is a test failure test/CodeGen/BPF/inline_asm.ll. Could you take a look?
Apr 19 2022
- create and use isLoadInst() helper function.
- use report_fatal_error instead of assert.
Apr 18 2022
The UseIntegratedAssembler = false is added with the following commit,
commit d2e5157c1f0b1953c5166c1d656ac71e840615a4 Author: Fangrui Song <i@maskray.me> Date: Sat Apr 11 10:01:36 2020 -0700
Apr 15 2022
Apr 14 2022
- simplify code with stripPointerCasts
Mar 29 2022
Ya, typo. memcpy is okay, it is memcmp. I probably just typing memcpy way more times than memcmp :-) Will change.
Mar 24 2022
Indeed, alignment is not used for union type. Your change looks good to me.
Mar 22 2022
LGTM. Maybe @efriedma can look at this as well.
Mar 21 2022
- fix typo
- add a new test case with typedef of volatile type.
Mar 17 2022
Mar 16 2022
Mar 15 2022
- only handle volatile variables with component assignment.
- change TransformBTFTagAttributedType() implementation with a simple llvm_unreachable() message. I tested linux kernel and its selftests/bpf compilation with clang and found the only TreeTransformation function is called with TransformAutoType() due to __auto_type usage.
@eli.friedman Please feel free adding other reviewers if needed. Thanks!
Mar 14 2022
- add one more _Generic test.
Mar 12 2022
- fix one 'auto' declaration issue
- rework on the test with overloadable attribute.
Mar 11 2022
Mar 9 2022
@aaron.ballman @erichkeane Hopefully I addressed your comments. Could you take a look again?
- use BTFTypeTagAttr instead of Attr in PropertiesBase.td
- further clang-format fixes.
- clang-format fix
- change variable names to be llvm compliant.
- remove Attr from BTFTagAttributedTypeLoc as it can be retrieved from the BTFTagAttributedType
- Also transform the attribute in TreeTransform
- add additional test with _Generic and overloadable attribute
@aaron.ballman Thanks for the review! I will address all your comments and will also add tests with _Generic and __attribute__((overloadable)).
Mar 8 2022
- add a pch test to test serialization with btf_type_tag.
- simplify the code
@erichkeane Just posted a patch to address your comments. Could you take a look? I will try to add more tests, esp. serialization and libclang. If you have any suggestions in these two areas (as I am not familiar with them), it would be great!
- address @erichkeane comments
- further TODO: add more tests
Mar 7 2022
@erichkeane Thanks for the review. I will try to address your comments as soon as possible.
Mar 5 2022
@aaron.ballman @erichkeane Just updated the patch with a new implementation, which added a new BTFTagAttributedType instead of reusing existing AttributedType. It is quite complicated and various parts of clang are touched. I mostly just followed what AttributedType did but considering BTFTagAttributedType is C only so my current implementation may unnecessarily complicate things. I didn't have much test coverage either. It would be great you can give a look and let me know (1). what I am missing, (2). where I can simplify things, (3) a different approach if you see fit, (4). what additional tests I should have. Thanks!
- Create a separate type BTFTagAttributed instead of using Attributed type based on previous discussion
Feb 25 2022
@erichkeane Thanks for suggestion. I will add as an independent type then.
@aaron.ballman Thanks for suggestion. Agree that Having a new Type is a better idea. I guess, I will add BTFTagAttributeType which extends AttributeType to see whether it works or not.
Feb 24 2022
@aaron.ballman ping, did you get time to look at this patch?
fix clang-format issues.