Today

phosek created D43606: [Driver] Add SafeStack to a map of incompatible sanitizers.
Wed, Feb 21, 8:34 PM
hintonda added a comment to D43601: [bugpoint] Add NoStripSymbols option..
In D43601#1015341, @vsk wrote:

A simpler way to do this might be to call 'strip' on the module as an final cleanup, just like we do for GlobalDCE.

Wed, Feb 21, 8:32 PM
hintonda updated the diff for D43601: [bugpoint] Add NoStripSymbols option..

Address comments.

Wed, Feb 21, 8:27 PM
skatkov added a comment to D43536: [LV] Quick workaround for PR36311, vectorizer's isUniform() abuse triggers assert in SCEV.

SGTM, the same disclaimer :) I'm not a very good at LoopVectorizer :)

Wed, Feb 21, 8:27 PM
Meinersbur added a comment to D43536: [LV] Quick workaround for PR36311, vectorizer's isUniform() abuse triggers assert in SCEV.

LGTM, however I am not the right person to approve patches to the LoopVectorizer.

Wed, Feb 21, 8:24 PM
mtrofin added a reviewer for D43605: [SampleProf] NFC. Expose reusable functionality in SampleProfile.: davidxl.
Wed, Feb 21, 8:24 PM
mtrofin created D43605: [SampleProf] NFC. Expose reusable functionality in SampleProfile..
Wed, Feb 21, 8:24 PM
emaste added a comment to D42516: [llvm-objcopy] Add support for large indexes.
Wed, Feb 21, 7:14 PM
hintonda added inline comments to D43601: [bugpoint] Add NoStripSymbols option..
Wed, Feb 21, 7:05 PM
zturner added a comment to D43060: [CodeView] Lower type for dwarf::DW_TAG_restrict_type type.

I should probably defer to @rnk for the main content of the review here.

Wed, Feb 21, 7:05 PM
nemanjai committed rL325739: [PowerPC] Do not produce invalid CTR loop with an FRem.
[PowerPC] Do not produce invalid CTR loop with an FRem
Wed, Feb 21, 7:04 PM
hintonda added a comment to D43601: [bugpoint] Add NoStripSymbols option..

Okay reading the code a bit more seems like that is what you are doing.

But we don't want user flags for bugpoint, we want it to try both variants and keep the smaller one when it still reproduces the crash.

Okay reading the code some more, it seems like that is what you are doing here and the option is just to disable the extra try.

  • You should split the OptionCategory changes into a separate commit.
Wed, Feb 21, 7:04 PM
rhysd added a comment to D43070: Add an instruction to avoid cgo compilation error from Go 1.9.4.

If we want this for 6.0, it needs to land real soon.
Eric, do you have concerns or do you want to commit it?

Wed, Feb 21, 7:02 PM
rhysd added a comment to D43070: Add an instruction to avoid cgo compilation error from Go 1.9.4.

It looks like they've been set into the default set here: https://go-review.googlesource.com/c/go/+/94676

Wed, Feb 21, 6:56 PM
vsapsai added a comment to D42498: [ExprConstant] Fix crash when initialize an indirect field with another field..

Thanks for response, Richard. I'll look into using CXXDefaultInitExpr.

Wed, Feb 21, 6:55 PM
STL_MSFT updated the diff for D43273: [libcxx] [test] Fix MSVC warnings and errors..

Update based on code review feedback.

Wed, Feb 21, 6:32 PM
jakehehrlich updated the diff for D42516: [llvm-objcopy] Add support for large indexes.

You can download the zip file that I'm using here: https://drive.google.com/file/d/1u6W1mUHkFBPsLzEV4u50M_BeiPKMyipM/view?usp=sharing

Wed, Feb 21, 6:29 PM
MatzeB added a comment to D43601: [bugpoint] Add NoStripSymbols option..

Okay reading the code a bit more seems like that is what you are doing.

Wed, Feb 21, 6:25 PM
zturner added a comment to D43165: [lit] Fix problem in how Python versions open files with different encodings.

I checked the code and this happens indirectly inside of a call to compareDirectoryTrees. So indeed, it would be hard to explicitly request a binary diff. Instead, could we detect if a file is binary, and if it is fall back to a binary diff? For example, using code like this. We could put this in a function named lit.util.is_binary(file) and inside of compareDirectoryTrees we could call this function and then branch to the appropriate diff function.

Thoughts?

There's also mimetypes.guess_type(file) which might be helpful here.

Wed, Feb 21, 6:22 PM
andrew.w.kaylor added inline comments to D43515: More math intrinsics for conservative math handling.
Wed, Feb 21, 6:18 PM
MatzeB added a comment to D43601: [bugpoint] Add NoStripSymbols option..

But we don't want user flags for bugpoint, we want it to try both variants and keep the smaller one when it still reproduces the crash.

Wed, Feb 21, 6:18 PM
jakehehrlich added a comment to D42516: [llvm-objcopy] Add support for large indexes.

I spent some time today figuring out how to a) improve the time it takes the test to run and b) make the uploaded binary as small as possible. I'm uploading a binary so that not every test has to regenerate the same file (which dominates the running time of generating this file). I tried to make this binary as small and as compressible as possible. I got the compressed archive down to 147 kb which I think is acceptable. On my machine the total running time of decompressing, running llvm-objcopy, and then checking the data, is about 1.6 seconds which is acceptably fast in my opinion. This problem was much more solvable than when I implemented 64-bit symbol offsets for archives.

Wed, Feb 21, 6:15 PM
efriedma added inline comments to D43594: [AMDGPU] Respect pragma unroll when loop contains convergent instructions.
Wed, Feb 21, 6:11 PM
dberlin added a comment to D40874: [LV][LoopInfo] Add irreducible CFG detection for outer loops.

Thanks, that is very helpful. It's much more likely that Dominator info is up to date than loop info, but it's not worth getting into here. The only thing I request is that you update the comments to suggest this only probably makes sense to use if loopinfo is computed already, and please just add your description of how it works with the example to the comments.

Wed, Feb 21, 6:11 PM
jakehehrlich added a comment to D42516: [llvm-objcopy] Add support for large indexes.

What error would you expect in this case? Why wouldn't it just work?

Wed, Feb 21, 6:05 PM
rnk added a comment to D43547: [NameMangling] Make ASTContext owning the ManglingContext during entire compilation.

Can you update the description to clarify that this is fixing a bug in the indexing library? From the description it sounds like we have a serious bug in FUNCDNAME codegen, which is not the case. CodeGen does the right thing. The ASTContext API is just crappy, so the Index library used it incorrectly. The fix is to simplify the ASTContext API so that it owns the mangling context.

Wed, Feb 21, 5:53 PM
tra added a comment to D43602: [CUDA] Added missing functions..

For my information, how are we verifying that we've caught everything?

Wed, Feb 21, 5:36 PM
vedantk committed rL325738: [Utils] Avoid a hash table lookup in salvageDI, NFC.
[Utils] Avoid a hash table lookup in salvageDI, NFC
Wed, Feb 21, 5:32 PM
tra added inline comments to D43559: TableGen: fix typeIsConvertibleTo for record types.
Wed, Feb 21, 5:25 PM
rtereshin added a comment to D43603: [utils] fixing update_mir_test_checks.py's greediness for `registers:` field.

I've also double checked that the test actually fails. On the original tool it fails with

-- Testing: 1 tests, 1 threads --
FAIL: LLVM :: CodeGen/AArch64/actual_test.mir (1 of 1)
******************** TEST 'LLVM :: CodeGen/AArch64/actual_test.mir' FAILED ********************
Script:
--
sed -e 's/^# RUN-NESTED:/# RUN:/' /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir | sed -e 's/^# SOURCE.*$//' > /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp    && /Volumes/Data/llvm/test/CodeGen/AArch64/../../../utils/update_mir_test_checks.py --add-vreg-checks      /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp 2>&1 | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=STDERR    && cat /Volumes/Data/llvm/build/obj/test/CodeGen/AArch64/Output/actual_test.mir.tmp | /Volumes/Data/llvm/build/obj/bin/FileCheck /Volumes/Data/llvm/test/CodeGen/AArch64/actual_test.mir --check-prefix=SOURCE
--
Exit Code: 1
Wed, Feb 21, 5:21 PM
jlebar accepted D43602: [CUDA] Added missing functions..

For my information, how are we verifying that we've caught everything?

Wed, Feb 21, 5:15 PM
tra accepted D43558: TableGen: Add strict assertions to sanity check earlier type checking.
Wed, Feb 21, 5:15 PM
rtereshin created D43604: [utils] fixing update_mir_test_checks.py's language (mir/IR/etc) detection.
Wed, Feb 21, 5:15 PM
rtereshin created D43603: [utils] fixing update_mir_test_checks.py's greediness for `registers:` field.
Wed, Feb 21, 5:12 PM
tra accepted D43557: TableGen: Allow implicit casting between string and code.
Wed, Feb 21, 5:11 PM
tra updated the diff for D43602: [CUDA] Added missing functions..

Added missing __threadfence_system().

Wed, Feb 21, 5:06 PM
tra created D43602: [CUDA] Added missing functions..
Wed, Feb 21, 5:04 PM
hintonda added a comment to D43601: [bugpoint] Add NoStripSymbols option..
In D43601#1015344, @vsk wrote:
In D43601#1015341, @vsk wrote:

A simpler way to do this might be to call 'strip' on the module as an final cleanup, just like we do for GlobalDCE.

So, always run it or use a flag?

Gating the behavior behind an off-by-default flag sgtm.

Wed, Feb 21, 5:04 PM
vsk added a comment to D43601: [bugpoint] Add NoStripSymbols option..
In D43601#1015341, @vsk wrote:

A simpler way to do this might be to call 'strip' on the module as an final cleanup, just like we do for GlobalDCE.

So, always run it or use a flag?

Wed, Feb 21, 4:58 PM
hintonda added a comment to D43601: [bugpoint] Add NoStripSymbols option..
In D43601#1015341, @vsk wrote:

A simpler way to do this might be to call 'strip' on the module as an final cleanup, just like we do for GlobalDCE.

Wed, Feb 21, 4:57 PM