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

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 2 07:47:42 PDT 2023


erichkeane added inline comments.


================
Comment at: llvm/test/CodeGen/BPF/preserve-static-offset/load-align.ll:61
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 18.0.0 (/home/eddy/work/llvm-project/clang c899a1ca75d0f1b559204eff79a2578d2cafc7ab)"}
+!2 = !{!3, !4, i64 128}
----------------
eddyz87 wrote:
> erichkeane wrote:
> > eddyz87 wrote:
> > > erichkeane wrote:
> > > > Are we sure we want to do something like this?  It seems this both depends on YOUR computer AND us never releasing Clang 18.
> > > Are you sure this would be an issue?
> > > The specific line is not a part of a CHECK and I tried the following command using my system's llvm 16 opt:
> > > 
> > > ```
> > > opt -O2 -mtriple=bpf-pc-linux -S -o - load-align.ll
> > > ```
> > > 
> > > And module was loaded / processed w/o any issues.
> > > In general grepping shows that people don't usually mask these in tests:
> > > 
> > > ```
> > > $ cd llvm/test/CodeGen/
> > > $ ag '{!"clang version' | wc -l
> > > 452
> > > ```
> > I don't write LLVM tests ever, so I'm not sure.  It just seems odd to provide that much irrelevant info, perhaps one of hte LLVM reviewers can comment.  Also, look at those ~450 and see what they contain?
> > Also, look at those ~450 and see what they contain?
> Same random clang versions:
> 
> ```
> $ ag '{!"clang version' | head
> X86/debug-loclists.ll:129:!6 = !{!"clang version 10.0.0 (trunk 374581) (llvm/trunk 374579)"}
> X86/dbg-combine.ll:84:!11 = !{!"clang version 3.7.0 (trunk 227074)"}
> X86/debuginfo-locations-dce.ll:46:!6 = !{!"clang version 8.0.0 (trunk 339665)"}
> X86/pr31242.ll:48:!5 = !{!"clang version 4.0.0 (trunk 288844)"}
> X86/catchpad-regmask.ll:140:!1 = !{!"clang version 3.8.0 "}
> X86/debug-nodebug-crash.ll:48:!5 = !{!"clang version 4.0.0"}
> X86/limit-split-cost.mir:64:  !2 = !{!"clang version 7.0.0 (trunk 335057)"}
> X86/swap.ll:169:!1 = !{!"clang version 9.0.0 (trunk 352631) (llvm/trunk 352632)"}
> X86/dwarf-aranges-available-externally.ll:65:!16 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 2f52a868225755ebfa5242992d3a650ac6aadce7)"}
> X86/label-annotation.ll:96:!7 = !{!"clang version 9.0.0 (git at github.com:llvm/llvm-project.git 7f9a008a2db285aca57bfa0c09858c9527a7aa98)"}
> ```
It seems at least removing your home-path would be a good idea, but I can't really review these either.

Note the 'trunk #####' is from our SVN days, so that isn't particularly useful at all.  IMO everything in the parens is worthless in the tests, but hopefully someone familiar with the LLVM tests can stop by and correct me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133361/new/

https://reviews.llvm.org/D133361



More information about the cfe-commits mailing list