[PATCH] D123300: [Clang] Enable opaque pointers by default

yurai007 added a comment. In D123300#3474834 , @nikic wrote: > @yurai007 I've put up https://reviews.llvm.org/D124459 to fix this > optimization failure. @nikic: I can confirm that patch fix regression and makes coroutines snippet great again. Thanks!

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @yurai007 I've put up https://reviews.llvm.org/D124459 to fix this optimization failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/ https://reviews.llvm.org/D123300

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. In D123300#3467615 , @yurai007 wrote: > Just one more thing regarding this: > > In D123300#3467165 , @yurai007 > wrote: > >> Hi, unfortunately for some reason it doesn't play well with

[PATCH] D123300: [Clang] Enable opaque pointers by default

yurai007 added a comment. Just one more thing regarding this: In D123300#3467165 , @yurai007 wrote: > Hi, unfortunately for some reason it doesn't play well with coroutines HALO. > There is regression seen on Gor's Nishanov classical code snippet: >

[PATCH] D123300: [Clang] Enable opaque pointers by default

markus added a comment. In D123300#3467309 , @nikic wrote: > Sounds reasonable in general -- though isn't this a pre-existing problem > you'd see if you simply had multiple loads from the same global (without any > GEP)? Looking at the current

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. In D123300#3467278 , @markus wrote: > We have run into a slight performance degrading issue with our downstream > target. The situation is that we relay on the "consthoist" pass with option > "-consthoist-gep=1" to factor out the

[PATCH] D123300: [Clang] Enable opaque pointers by default

markus added a comment. We have run into a slight performance degrading issue with our downstream target. The situation is that we relay on the "consthoist" pass with option "-consthoist-gep=1" to factor out the common parts of GEP expresseions to reduce the number of symbol references. For

[PATCH] D123300: [Clang] Enable opaque pointers by default

yurai007 added a comment. Hi, unfortunately for some reason it doesn't play well with coroutines HALO. There is regression seen on Gor's Nishanov classical code snippet: https://godbolt.org/z/PKMxqq4Gr I'm checking IR to find out more. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D123300: [Clang] Enable opaque pointers by default

markus added a comment. In D123300#3459023 , @nikic wrote: > @markus Without tracing through it in detail, I'd guess that without opaque > pointers this creates two getelementptr constant expressions that get folded > together. With opaque pointers,

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @markus Without tracing through it in detail, I'd guess that without opaque pointers this creates two getelementptr constant expressions that get folded together. With opaque pointers, the first one (which would be a zero-index GEP) is omitted, and only the second one is

[PATCH] D123300: [Clang] Enable opaque pointers by default

markus added a comment. With $ ./bin/clang -cc1 -emit-llvm BBI-68673.c the IR contains the following @a2_ldm_i64 = global [4 x [18 x { i64, i64 }]] zeroinitializer, align 16 %arrayidx = getelementptr inbounds [4 x [18 x { i64, i64 }]], ptr @a2_ldm_i64, i64 0, i64 %idxprom %.real =

[PATCH] D123300: [Clang] Enable opaque pointers by default

aeubanks added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/ https://reviews.llvm.org/D123300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D123300: [Clang] Enable opaque pointers by default

fhahn added a comment. In D123300#3454215 , @aeubanks wrote: > $ cat /tmp/a.ll > target triple = "thumbv8-unknown-linux-gnueabihf" > > define void @zot() { > bb: > br label %bb1 > > bb1: ;

[PATCH] D123300: [Clang] Enable opaque pointers by default

aeubanks added a comment. $ cat /tmp/a.ll target triple = "thumbv8-unknown-linux-gnueabihf" define void @zot() { bb: br label %bb1 bb1: ; preds = %bb1, %bb %tmp = phi ptr [ null, %bb ], [ %tmp2, %bb1 ] store ptr %tmp, ptr

[PATCH] D123300: [Clang] Enable opaque pointers by default

manojgupta added a comment. We noticed a new crash that still repros at head. clang -target armv7a-linux-gnueabihf -march=armv8a -mthumb -c -O2 file.cc llvm-project/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10135: llvm::Value *llvm::VPTransformState::get(llvm::VPValue *, unsigned int):

[PATCH] D123300: [Clang] Enable opaque pointers by default

uabelho added a comment. In D123300#3451087 , @nikic wrote: > @uabelho The IR is correct, but requires using `opt -opaque-pointers` > explicitly. Normally, opaque pointer mode is automatically enabled, but there > is no explicit mention of `ptr` in the

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @uabelho The IR is correct, but requires using `opt -opaque-pointers` explicitly. Normally, opaque pointer mode is automatically enabled, but there is no explicit mention of `ptr` in the IR. Not sure if we can do anything to improve that before the default on the opt

[PATCH] D123300: [Clang] Enable opaque pointers by default

uabelho added a comment. Hi, I noticed this produces broken code: clang -cc1 -triple amdgcn-- -emit-llvm -o - -x c op.c with op.c being void bar(); void foo() { bar(); } The result is define dso_local void @foo() #0 { entry: call void @bar(i32 noundef 42) ret void

[PATCH] D123300: [Clang] Enable opaque pointers by default

mstorsjo added a comment. In D123300#3446823 , @aeubanks wrote: > In D123300#3446023 , @nikic wrote: > >> @mstorsjo Thanks! I've reduced this to a crash in `-argpromotion`: >> >> efine void @caller() { >>

[PATCH] D123300: [Clang] Enable opaque pointers by default

paulwalker-arm added a comment. We've spotted some breakages caused by this patch within the llvm test suite when built for AArch64-SVE. I've got https://reviews.llvm.org/D123670 as a WIP fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123300: [Clang] Enable opaque pointers by default

aeubanks added a comment. In D123300#3447116 , @maryammo wrote: > @nikic, it seems breaking the clang-ppc64le-rhel build > > htps://lab.llvm.org/buildbot/#/builders/57/builds/16776 > > Can you please take a look? should be fixed by

[PATCH] D123300: [Clang] Enable opaque pointers by default

maryammo added a comment. @nikic, it seems breaking the clang-ppc64le-rhel build htps://lab.llvm.org/buildbot/#/builders/57/builds/16776 Can you please take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/

[PATCH] D123300: [Clang] Enable opaque pointers by default

aeubanks added a comment. In D123300#3446023 , @nikic wrote: > @mstorsjo Thanks! I've reduced this to a crash in `-argpromotion`: > > efine void @caller() { > call i32 @callee(ptr null) > ret void > } > > define internal void @callee(ptr

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @mstorsjo Thanks! I've reduced this to a crash in `-argpromotion`: efine void @caller() { call i32 @callee(ptr null) ret void } define internal void @callee(ptr %p) { ret void } Similar issue with function type mismatch. Repository: rG LLVM

[PATCH] D123300: [Clang] Enable opaque pointers by default

vitalybuka added a comment. In D123300#3444988 , @nikic wrote: > @vitalybuka Are there any instructions on how to reproduce failures from this > buildbot? It seems like this needs more than a simple bootstrap build due to > the need for instrumented

[PATCH] D123300: [Clang] Enable opaque pointers by default

mstorsjo added a comment. In D123300#3444903 , @nikic wrote: > @mstorsjo Thanks for the report, the issue should be fixed by > https://github.com/llvm/llvm-project/commit/8d5c8d57c637d898094af323d1888ea5a3364f8c. Awesome, thanks! Unfortunately, there

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. I've put up https://reviews.llvm.org/D123602 to fix the msan issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/ https://reviews.llvm.org/D123300 ___ cfe-commits

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. Okay, I managed to reproduce this using the instructions from https://github.com/google/sanitizers/wiki/MemorySanitizerBootstrappingClang. Reduced to these two variants for `-passes=msan`: target triple = "x86_64-unknown-linux-gnu" define void @test(i8* %p, i32*

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @vitalybuka Are there any instructions on how to reproduce failures from this buildbot? It seems like this needs more than a simple bootstrap build due to the need for instrumented libcxx? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123300: [Clang] Enable opaque pointers by default

nikic added a comment. @mstorsjo Thanks for the report, the issue should be fixed by https://github.com/llvm/llvm-project/commit/8d5c8d57c637d898094af323d1888ea5a3364f8c. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/

[PATCH] D123300: [Clang] Enable opaque pointers by default

mstorsjo added a comment. This triggered (or exposed) assertion failures: $ cat inlinecost.cpp typedef char a; typedef unsigned b; typedef int c; typedef struct { b d[]; } e; e f(void *) {} typedef void *fptr; fptr g; template void h(fptr i) { int (*j)(const void *)

[PATCH] D123300: [Clang] Enable opaque pointers by default

vitalybuka added a comment. In D123300#3443603 , @vitalybuka wrote: > https://lab.llvm.org/buildbot/#/builders/74/builds/10288/steps/14/logs/stdio This looks like a false report. Probably opaque pointers somehow break Msan. Repository: rG LLVM

[PATCH] D123300: [Clang] Enable opaque pointers by default

vitalybuka added a comment. https://lab.llvm.org/buildbot/#/builders/74/builds/10288/steps/14/logs/stdio FAILED: tools/clang/lib/Tooling/ASTNodeAPI.json cd /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan_track_origins/tools/clang/lib/Tooling &&

[PATCH] D123300: [Clang] Enable opaque pointers by default

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG702d5de4380b: [Clang] Enable opaque pointers by default (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits.