[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: llvm/lib/IR/IRBuilder.cpp:1021
   Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC));
   return CreateShuffleVector(V, Undef, Zeros, Name + ".splat");
 }

nlopes wrote:
> while at it, don't you want to change this one to poison as well?
It would be great, but there are many other places that create shufflevector 
with undef args (InstCombineCalls, etc). Since this and related patches are big 
changes, I think it might be good to land these first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

I'll make two more patches - the instsimplify/vectorizer/ changes that make 
insertelement poison, and the langref update to shufflevector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93586

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93806: [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

It would be great if we had some spec for this style, it gets a bit hard to 
verify if the formatting is actually what is expected.
Having said that, LGTM if the bug reporter confirms the behaviour is ok.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2248
+
+  bool RemoveWhiteSmithCaseIndent =
+  (!Style.IndentCaseBlocks &&

Nit: the style is named `Whitesmiths` so I'd expect the variable to match that: 
small s in the middle, trailing s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93806

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added subscribers: balazske, vabridgers.
vabridgers added a comment.

I was referred to this patch from https://reviews.llvm.org/D86743. I pulled 
this patch under review, brought it up to date and pushed to github at 
https://github.com/vabridgers/llvm-project-dev.git, branch: vla-fam-fixes. 
Everything seems ok on this branch (LITs pass,  reproducers from 
https://bugs.llvm.org/show_bug.cgi?id=47272 and 
https://bugs.llvm.org/show_bug.cgi?id=28450 no longer crash). I can continue 
and push a change to Phabricator for review, or @charusso and/or @balazske 
could finish this? I didn't want to just push an update without asking first :/ 
 Cheers!


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

https://reviews.llvm.org/D69726

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I pulled https://reviews.llvm.org/D69726, brought it up to date and pushed to 
github at https://github.com/vabridgers/llvm-project-dev.git, branch: 
vla-fam-fixes. Everything seems ok on this branch (LITs pass, reproducers from 
https://bugs.llvm.org/show_bug.cgi?id=47272 and 
https://bugs.llvm.org/show_bug.cgi?id=28450 no longer crash). I think perhaps 
this change can be abandoned in favor of progressing 
https://reviews.llvm.org/D69726 ? @charusso and @balazske , what do you think? 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93787: [analyzer] Fix crash caused by accessing empty map

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a reviewer: steakhal.
steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

This form of macro expansion is obsolete - I hope that the community agrees on 
this.
Crashes for many more cases then just the one you mentioned. Its a really hard 
and bugrone to mimic the preprocessor, thus we seek to substitute this with the 
clang's preprocessor implementation.
I recommend pushing an XFAIL lit test demonstrating the current limitation 
(without your proposed fix), while we can show that this will pass after we 
accept my patches.
For the record those are in an early phase at D93222 
.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1241
   if (TheTok.getIdentifierInfo() == VariadicParamII) {
-TStream.injectRange(PrevParamMap.at(VariadicParamII));
+if (PrevParamMap.find(VariadicParamII) != PrevParamMap.end())
+  TStream.injectRange(PrevParamMap.at(VariadicParamII));

Btw this lookup supposed to be successful. Always. Which suggests me that there 
are even more logic bug lurking there.
Without using 'at' here we wouldn't notice it, which is lucky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93787

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits