[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

yonghong-song added a comment. I tried a couple of examples with both preserve_access_index and preserve_static_offset (both '__preserve_access_index __preserve_static_offset' and '__preserve_static_offset __preserve_access_index'). Looks they cooperate with each other properly.

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

yonghong-song added a comment. I went through LLVM part of the change, mostly look good to me from high-level and I need to go through another pass with details. It would be great if you can post corresponding kernel patches which utilizes this functionality? Comment at:

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

yonghong-song added a comment. > I can rename these things, but tbh I don't think this functionality would be > useful anywhere outside BPF, thus such renaming would be kind-of deceptive > (and in case it would be useful, the renaming could be done at the time of > second use). Then let us

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

yonghong-song added a comment. I will try to review other parts of code in the next few days. Comment at: llvm/include/llvm/IR/Intrinsics.td:2432 ImmArg>]>; +def int_context_marker_bpf : DefaultAttrsIntrinsic<[llvm_ptr_ty], +

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

yonghong-song added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:2432 ImmArg>]>; +def int_context_marker_bpf : DefaultAttrsIntrinsic<[llvm_ptr_ty], +

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

yonghong-song added a comment. > Ah, right right - sorry, failed to push. Here it is: > 2993243c45abdb4f2bc3979336d054be165b1134 > Thanks! I tested and the problem with bpf backend is resolved! Repository: rG LLVM Github

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

yonghong-song added a comment. In D156726#4593671 , @dblaikie wrote: > Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a Hi, @dblaikie I cannot find the above commit in llvm-project and with latest llvm-project 'main' branch, the problem seems still

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6c412b6c6faa: [BPF] Add a few new insns under cpu=v4 (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 544055. yonghong-song added a comment. - Add more tests in assembler-disassembler-v4.s and gotol.ll. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/ https://reviews.llvm.org/D144829 Files:

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 543709. yonghong-song added a comment. Three major changes in this patch: - for ldsx insns, remove 32bit ldsx insns (1-byte and 2-byte sign extension) since the ldsx insn expects to sign extension all the way up to 8-byte and normal 32bit insn (e.g.

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

yonghong-song added a comment. > Could you please also add a few tests for `gotol`? Will do! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/ https://reviews.llvm.org/D144829 ___ cfe-commits

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

yonghong-song added inline comments. Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:379 + "$dst = (s8)$src", + [(set GPR:$dst, (sra (shl GPR:$src, (i64 56)), (i64 56)))]>; + def MOVSX_rr_16 : ALU_RR I think it is possible to avoid

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

yonghong-song added a comment. In D144829#4519036 , @eddyz87 wrote: > I tried adding a test similar to `assemble-disassemble.ll`: > > // RUN: llvm-mc -triple bpfel --mcpu=v4 --assemble --filetype=obj %s \ > // RUN: | llvm-objdump -d --mattr=+alu32

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 542245. yonghong-song added a comment. - remove a copy-paste comment from s390 arch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/ https://reviews.llvm.org/D144829 Files:

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 541265. yonghong-song edited the summary of this revision. yonghong-song added a comment. - Dropping 'CPUv4' in some variable/function names and also in debug flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song added inline comments. Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:56 def BPFNoALU32 : Predicate<"!Subtarget->getHasAlu32()">; +def BPFHasCPUv4_ldsx : Predicate<"Subtarget->getCPUv4_ldsx()">; +def BPFHasCPUv4_movsx : Predicate<"Subtarget->getCPUv4_movsx()">;

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 539842. yonghong-song edited the summary of this revision. yonghong-song added a comment. - rename some insn names or mode names (movs -> movsx, lds -> ldsx, MEMS -> MEMSX) etc to be consistent with kernel. - add 5 llc flags to control on/off for each

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song added a comment. In D144829#4465193 , @eddyz87 wrote: > Hi Yonghong, > > What is the current plan for these changes? > I'd like to refresh D140804 to make BPF_ST > instruction available for cpu v4. > I

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

yonghong-song accepted this revision. yonghong-song added a comment. I am okay with this change. Once you used this patch and implemented the mechanism inside Google. Could you send a follow-up summary on what the implementation looks like in Google? This will give people a sense why this is

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 534801. yonghong-song added a comment. - avoid changing conditions during JMP -> JMPL conversion. Otherwise, verification may fail in some cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 534361. yonghong-song added a comment. - added support of new instructions in inline assembly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/ https://reviews.llvm.org/D144829 Files:

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 523569. yonghong-song edited the summary of this revision. yonghong-song added a comment. - Fixed previous llvm-objdump issue for '> 16bit' 'gotol' insns. - Now basic functionality for cpu=v4 should be complete for llvm, further work will focus on

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song updated this revision to Diff 523253. yonghong-song added a comment. - Fixed issues reported by Eduard - llvm-objdump issue (as stated in 'Summary') is not resolved yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144829/new/

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

yonghong-song added a comment. @dblaikie could you help take a look at this patch? Similar to https://reviews.llvm.org/D143966, this patch is the clang frontend change to support new btf_type_tag format, as we discussed and agreed with gcc community

[PATCH] D150017: [Clang][BPF] Type print btf_type_tag properly

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3060304906f0: [Clang][BPF] Type print btf_type_tag properly (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D150017: [Clang][BPF] Type print btf_type_tag properly

yonghong-song added a comment. In D150017#4326665 , @aaron.ballman wrote: > LGTM, but please also add a release note about the fix. Thanks @aaron.ballman. I intend to backport this patch to llvm16 and adding a release note about the fix will have

[PATCH] D150017: [Clang][BPF] Type print btf_type_tag properly

yonghong-song created this revision. yonghong-song added reviewers: ast, aaron.ballman. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When running bcc tool execsnoop ([1]) which is built with

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

yonghong-song added a comment. I see the following in the Summary: Type tag for CVR modifier type C: volatile int __attribute__((btf_type_tag("__b"))) b; DWARF: 0x31: DW_TAG_variable DW_AT_name ("b") DW_AT_type (0x3c "volatile int")

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

yonghong-song added a comment. Related to `btf_type_is_modifier()` issue, Not that depending on call site, sometimes typedef is skipped and sometimes are not. So we could keep btf_type_is_modifier() as is and modify the call size to not skip typedef in specific places. Or we could remove

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

yonghong-song created this revision. Herald added a subscriber: hiraditya. Herald added a project: All. yonghong-song requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. In [1], a few new insns are proposed to exgend BPF ISA

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

yonghong-song added a comment. @dyung I just pushed the fix to the 'main' branch (https://github.com/llvm/llvm-project/commit/183d075055c591dedead7ece972f1bdea611aa3b). Please check it out. Thanks for reporting! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56b038f887f3: [BPF][clang] Ignore stack protector options for BPF target (authored by eddyz87, committed by yonghong-song). Repository: rG LLVM

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

yonghong-song added a comment. Sorry, I double checked. '-fstack-protector -fno-stack-protector' will not result in warnings. So the patch LGTM. So gentoo people can add -fno-stack-protector to suppress warnings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

yonghong-song added a comment. @ast With this patch, gentoo clang compilation will hit the warning even if people appends -fno-stack-protector. Is this okay? In general, if the option is '-fstack-protector -fno-stack-protector', we should not issue warning, right? Repository: rG LLVM Github

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

yonghong-song added a comment. @compnerd could you also take a look at this patch? First, some background about this patch. The reason of this patch is due to: https://lore.kernel.org/bpf/CAOFdcFPnHEc2qd-=c+hdk4ntjjfbhsf4r-g7pdjtrbat6mu...@mail.gmail.com/ Further the following link has

[PATCH] D141424: [clang][Sema] Fix uninitialized `SourceLocation` for types with multiple attributes and macros.

yonghong-song accepted this revision. yonghong-song added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141424/new/ https://reviews.llvm.org/D141424

[PATCH] D140970: [BPF] preserve btf_decl_tag for parameters of extern functions

This revision was automatically updated to reflect the committed changes. Closed by commit rG58bdf8f9a8c3: [BPF] preserve btf_decl_tag for parameters of extern functions (authored by eddyz87, committed by yonghong-song). Changed prior to commit:

[PATCH] D140970: [BPF] preserve btf_decl_tag for parameters of extern functions

yonghong-song accepted this revision. yonghong-song added a comment. This revision is now accepted and ready to land. LGTM. I will add `// REQUIRES: bpf-registered-target` in the test to ensure it only executed with bpf target. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140929: [BPF] Support for "btf_decl_tag" annotations for arguments of extern functions

yonghong-song added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4218 + // DISubprogram's retainedNodes in the subsequent finalizeSubprogram call. + if (IsDeclForCallSite && CGM.getTarget().getTriple().isBPF()) { +if (auto *FD = dyn_cast(D)) {

[PATCH] D140929: [BPF] Support for "btf_decl_tag" annotations for arguments of extern functions

yonghong-song added a comment. Please separate the patch to clang and llvm part. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4218 + // DISubprogram's retainedNodes in the subsequent finalizeSubprogram call. + if (IsDeclForCallSite && CGM.getTarget().getTriple().isBPF()) {

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f047b56ba0e: [clang][Sema] Fix a clang crash with btf_type_tag (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

yonghong-song updated this revision to Diff 472305. yonghong-song added a comment. - improve the bug description in release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136807/new/ https://reviews.llvm.org/D136807 Files:

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

yonghong-song added a comment. @aaron.ballman or @dblaikie I have addressed the comments, could you take a look again? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136807/new/ https://reviews.llvm.org/D136807

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

This revision was automatically updated to reflect the committed changes. Closed by commit rG524c640090a8: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names (authored by eddyz87, committed by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

yonghong-song updated this revision to Diff 471216. yonghong-song added a comment. - Add the bug fix description in Release Note - Actually compare debuginfo btf_type_tag output instead of marking 'expected-no-diagnostics'. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

yonghong-song added a comment. In D136807#3888317 , @aaron.ballman wrote: > The changes generally look good to me, but can you add a release note for the > fix as well? Yes, will add the bug fix brief description to clang/docs/ReleaseNotes.rst

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

yonghong-song created this revision. yonghong-song added reviewers: aaron.ballman, dblaikie. yonghong-song added a project: clang. Herald added a project: All. yonghong-song requested review of this revision. Herald added a subscriber: cfe-commits. For the following program, $ cat t.c

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

yonghong-song added a comment. In D136041#3863748 , @dblaikie wrote: > Hmm - this does mean linking IR can produce invalid code, though, right (you > link in a definition of the function, so what was valid is now invalid - > because it now has a

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

yonghong-song added a comment. @aprantl The following example shows the difference between gcc and clang w.r.t. reserved names. [$ ~/tmp3] cat t.c extern void *__bpf_kptr_new(int, int, void *) __attribute__((section(".ksyms"))); #define bpf_kptr_new(x) __bpf_kptr_new(x, 0, 0)

[PATCH] D134705: [clang][DebugInfo] Emit debuginfo for non-constant case value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG75be0482a2e2: [clang][DebugInfo] Emit debuginfo for non-constant case value (authored by yonghong-song). Repository: rG LLVM Github Monorepo

[PATCH] D134705: [clang][DebugInfo] Emit debuginfo for non-constant case value

yonghong-song updated this revision to Diff 463643. yonghong-song added a comment. - simplify test case, add more CHECK's and add comments to explain the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134705/new/

[PATCH] D134705: [clang][DebugInfo] Emit debuginfo for non-constant case value

yonghong-song added a comment. In D134705#3818657 , @dblaikie wrote: > SGTM - could you also add some CHECKs in the test that show the enumeration > was added to the enumerators list on the CU (& you can probably test with > just one enumerator - don't

[PATCH] D134705: [clang][DebugInfo] Emit debuginfo for non-constant case value

yonghong-song created this revision. yonghong-song added a reviewer: dblaikie. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, clang does not emit debuginfo for the switch stmt case

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG481d67d310a7: [Clang][BPF] Support record argument with direct values (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song updated this revision to Diff 453835. yonghong-song added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. add two llvm/test/CodeGen/BPF tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132144/new/

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song added a comment. > but if the function is always_inline it can have more than 5 arguments, > right? That is correct. Will add a test case for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132144/new/

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song added a comment. > Kernel changes were assuming at most 2 My new version code allows all 5 parameters to be a structure argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132144/new/ https://reviews.llvm.org/D132144

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song added a comment. The compiler only counted the total number of registers won't exceed 5. You could have 5 struct arguments (each one register), or 2 struct arguments (with two registers each) and one integer register. Whether to pass by value or by reference is a ABI issue, not

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song added a comment. In D132144#3732685 , @anakryiko wrote: > Are there any restrictions about number of struct arguments that can be > passed in? Kernel changes were assuming at most 2, should we have a test that > tests passing 3 structs

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

yonghong-song created this revision. yonghong-song added a reviewer: ast. Herald added subscribers: pengfei, kristof.beyls. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, record

[PATCH] D131598: [Clang][BPF]: Force sign/zero extension for return values in caller

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd9198f64d9be: [Clang][BPF]: Force sign/zero extension for return values in caller (authored by yonghong-song). Repository: rG LLVM Github

[PATCH] D131598: [Clang][BPF]: Force sign/zero extension for return values in caller

yonghong-song created this revision. yonghong-song added reviewers: ast, anakryiko. Herald added subscribers: pengfei, kristof.beyls. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently bpf

[PATCH] D131468: [WIP][BPF]: Force sign/zero extension for arguments in callee and return values in caller

yonghong-song added a comment. In D131468#3708977 , @pengfei wrote: > D124435 is going to change the assumption > :) Thanks for the pointer. So looks like there is an effort to always do sign/zero-extension in

[PATCH] D131468: [WIP][BPF]: Force truncation for arguments in callee and return values in caller

yonghong-song created this revision. yonghong-song added reviewers: ast, anakryiko. Herald added a subscriber: pengfei. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For function arguments,

[PATCH] D126839: [clang][BPF] Update comment to include TYPE_MATCH

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG70557eb3938a: [clang][BPF] Update comment to include TYPE_MATCH (authored by d-e-s-o, committed by yonghong-song). Repository: rG LLVM Github

[PATCH] D126839: [clang][BPF] Update comment to include TYPE_MATCH

yonghong-song added a comment. In D126839#3554474 , @d-e-s-o wrote: > In D126839#3554125 , @yonghong-song > wrote: > >> Also let us merge it unti kernel side change is in reasonable shape. > > From what I

[PATCH] D126839: [clang][BPF] Update comment to include TYPE_MATCH

yonghong-song added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126223: [Clang][Sema] Fix a SemaType/VisitTagTypeLoc assertion

yonghong-song abandoned this revision. yonghong-song added a comment. @compnerd provided a better fix https://reviews.llvm.org/D126093, so I am going to close this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126223/new/

[PATCH] D126093: Sema: adjust assertion to account for deduced types

yonghong-song accepted this revision. yonghong-song added a comment. This revision is now accepted and ready to land. 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

[PATCH] D126223: [Clang][Sema] Fix a SemaType/VisitTagTypeLoc assertion

yonghong-song created this revision. yonghong-song added reviewers: compnerd, aaron.ballman. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Patch [1] added btf_type_tag attribute support. In

[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

yonghong-song added a comment. @compnerd Thanks for reporting. I will take a look as soon as possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99/new/ https://reviews.llvm.org/D99 ___

[PATCH] D124813: [HLSL] Add clang builtin for HLSL.

yonghong-song resigned from this revision. yonghong-song added a comment. please having proper reviewers. Me, @anakryiko @ast are not the right persons to review this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124813/new/

[PATCH] D122271: [Clang] -Wunused-but-set-variable warning - handle also pre/post unary operators

yonghong-song added a comment. LGTM. Maybe @efriedma can look at this as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122271/new/ https://reviews.llvm.org/D122271 ___ cfe-commits mailing list

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

This revision was automatically updated to reflect the committed changes. Closed by commit rGbdf69f63df2c: [Clang] Fix an unused-but-set-variable warning with volatile variable (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

yonghong-song updated this revision to Diff 417060. yonghong-song added a comment. - fix typo - add a new test case with typedef of volatile type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121715/new/ https://reviews.llvm.org/D121715 Files:

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

yonghong-song added a comment. In D121715#3397407 , @efriedma wrote: > In D121715#3397215 , @xbolva00 > wrote: > >> If there is j = j + 1 .. do we warn ? > > > > In D121715#3397237

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

yonghong-song added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:2033 return; + unsigned DiagID = isa(VD) ? diag::warn_unused_but_set_parameter efriedma wrote: > (Accidental change?) Right. Thanks for catching this. This is a leftover from my

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

This revision was automatically updated to reflect the committed changes. Closed by commit rG3251ba2d0fcf: [Attr] Fix a btf_type_tag AST generation (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

yonghong-song added inline comments. Comment at: clang/test/Sema/warn-unused-but-set-variables.c:66 + for (int i = 0; i < 1000; i++) +j += 1; + return; efriedma wrote: > The handling of this testcase without your patch seems fine. Even if the > value of

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with volatile variable

yonghong-song updated this revision to Diff 415665. yonghong-song retitled this revision from "[Clang] Fix an unused-but-set-variable warning with compond assignment" to "[Clang] Fix an unused-but-set-variable warning with volatile variable". yonghong-song edited the summary of this revision.

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 415638. yonghong-song added a comment. - 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

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:6872 + const BTFTagAttributedType *oldType = TL.getTypePtr(); + StringRef Tag = oldType->getTag(); + QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with compond assignment

yonghong-song added a comment. @eli.friedman Please feel free adding other reviewers if needed. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121715/new/ https://reviews.llvm.org/D121715 ___

[PATCH] D121715: [Clang] Fix an unused-but-set-variable warning with compond assignment

yonghong-song created this revision. yonghong-song added a reviewer: eli.friedman. Herald added a project: All. yonghong-song requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For the following code, void test() { int j = 0;

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 415326. yonghong-song added a comment. - add one more _Generic test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 Files: clang/include/clang-c/Index.h

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added inline comments. Comment at: clang/include/clang/AST/TypeLoc.h:923 + void initializeLocal(ASTContext , SourceLocation loc) {} + + QualType getInnerType() const { return getTypePtr()->getWrappedType(); } yonghong-song wrote: > aaron.ballman

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added inline comments. Comment at: clang/include/clang/AST/TypeLoc.h:923 + void initializeLocal(ASTContext , SourceLocation loc) {} + + QualType getInnerType() const { return getTypePtr()->getWrappedType(); } aaron.ballman wrote: > yonghong-song

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 414872. yonghong-song added a comment. - fix one 'auto' declaration issue - rework on the test with overloadable attribute. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 Files:

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added inline comments. Comment at: clang/include/clang/AST/TypeLoc.h:923 + void initializeLocal(ASTContext , SourceLocation loc) {} + + QualType getInnerType() const { return getTypePtr()->getWrappedType(); } aaron.ballman wrote: > Do we also

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added a comment. @aaron.ballman @erichkeane Hopefully I addressed your comments. Could you take a look again? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 414267. yonghong-song added a comment. - use BTFTypeTagAttr instead of Attr in PropertiesBase.td Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 Files:

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 414263. yonghong-song added a comment. - further clang-format fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 Files: clang/include/clang-c/Index.h

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 414252. yonghong-song added a comment. - 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 -

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added a comment. @aaron.ballman Thanks for the review! I will address all your comments and will also add tests with `_Generic` and `__attribute__((overloadable))`. Comment at: clang/include/clang/AST/Type.h:4797 + + BTFTagAttributedType(QualType canon,

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 413995. yonghong-song added a comment. - add a pch test to test serialization with btf_type_tag. - simplify the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added a comment. @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! Repository: rG

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song updated this revision to Diff 413861. yonghong-song added a comment. - address @erichkeane comments - further TODO: add more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 Files:

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added a comment. @erichkeane Thanks for the review. I will try to address your comments as soon as possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added inline comments. Comment at: clang/include/clang/AST/Type.h:4793 + + QualType ModifiedType; + QualType EquivalentType; erichkeane wrote: > I suspect both of these aren't necessary. If the intent here is to just wrap > a single type, I

[PATCH] D120296: [Attr] Fix a btf_type_tag AST generation bug

yonghong-song added a comment. @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

  1   2   3   4   5   >