Today

dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

ThreadSanitizer: race on Swift array
ThreadSanitizer: race on Swift map

Unfortunately, we won't be able to tell the type of the variable (at least not now). I've updated the patch to say "race on Swift variable", but that's not really exact and it's not describing the problem well. Swift is getting exact threading rules that are very different from C/C++ and I'm trying to focus on making the reports readable and understandable by Swift users.

Is there a way we could still special-case the Swift reports? I understand the desire to keep the API generic, but I think we need to make sure the users understand the report. E.g. "Swift access race" much better describes what the problem is, and users words that define the Swift threading model.

Tue, Apr 25, 5:35 AM · Restricted Project
arichardson created D32484: [ELF] Improve error message for incompatible section flags.
Tue, Apr 25, 5:32 AM · lld
slthakur accepted D32004: [mips] Rework a portion of MipsCC interface. (NFC).

LGTM

Tue, Apr 25, 5:31 AM
JonasToth added inline comments to D32346: [clang-tidy] New readability check for strlen argument.
Tue, Apr 25, 5:30 AM
dorit added a comment to D30041: [PSCEV] Create AddRec for Phis in cases of possible integer overflow, using runtime checks.

Hi Sanjoy, I will upload a new fixed version soon; just have two quick followup questions below. Thanks a lot!
Dorit

Tue, Apr 25, 5:30 AM
RKSimon added a comment to D32277: Replace slow LEA instructions in X86.

D32352 is looking at more aggressive conversion of IMUL to multiple LEA instructions.

Thanks for notifying, the patch does not contain any 3-Operand LEAs as far as I can see, the only case we need to be careful with is for 2-Operand LEA with RBP/R13/EBP as a base register, since this is determined only after RA, I am thinking it's better to let my patch fix those cases rather than preventing that patch from running on the problematic targets

Sorry for being pedantic about the naming, but for AMD architectures the 'slowlea' cases (whether it uses the ALU or AGU pipe) include scale != 1 (even for 2-ops), but it doesn't seem to be noticeably affected by RBP/R13/EBP. Hence my interest in PR32326 to try and make it easier to discriminate.

will separating this feature form the existing slowLEA feature which is used in SLM (and giving it another name) make it less confusing?

Tue, Apr 25, 5:29 AM
RKSimon accepted D32162: Inline asm 0bH conflict.

LGTM

Tue, Apr 25, 5:27 AM
slthakur updated the diff for D30678: [MIPS] Fix uitofp and fptoui for vector and scalar types.

Addressed review comments

Tue, Apr 25, 5:26 AM
sdardis updated the diff for D25866: [Sema] Support implicit scalar to vector conversions.

Rebase + ping.

Tue, Apr 25, 5:24 AM
dvyukov added a comment to D31630: [tsan] Detect races on modifying accesses in Swift code.

Also, could swift runtime register a tag at startup? Or performance overhead is a concern?

It's quite hard to make the runtime call the registration at process startup, because it would need to dynamically figure out whether we're running with TSan or not, plus we'd need to add an indirection to each access (to read the value of the tag). I'd really prefer to keep the value as a well-defined constant in TSan.

Tue, Apr 25, 5:23 AM · Restricted Project
dvyukov added inline comments to D32382: [tsan] Track external tags in thread traces.
Tue, Apr 25, 5:21 AM · Restricted Project
krytarowski added a comment to D32421: Fix segfault resulting from empty print prompt.

I cannot reproduce it locally.

Tue, Apr 25, 5:20 AM
salari01 abandoned D31496: Make -defsym a driver option.
In D31496#721262, @rnk wrote:

-Wl,--defsym is also a pretty common linker option. I'm hesitant to add this to the driver interface, since it seems like they could easily be confused.

Tue, Apr 25, 5:19 AM
avt77 updated the diff for D32162: Inline asm 0bH conflict.

I inhibitted the default constructor for ParseStatementInfo::ParseStatementInfo() because it can't work without proper initialization.

Tue, Apr 25, 5:17 AM
Serge_Preis added a comment to D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195).

Is it possible to add a test for this change?
There are some source range checks in test/Misc/ast-dump-decl.cpp

Tue, Apr 25, 5:11 AM
kparzysz added a comment to D32117: Update TableGen LangIntro.rst.

Also, does the document mention expressions like "Inst{31,27-25,14}", where the bit accesses are in a comma-separated list?

Tue, Apr 25, 4:57 AM
danielmarjamaki added inline comments to D30691: [analyzer] Support for naive cross translational unit analysis.
Tue, Apr 25, 4:56 AM
asb added a reviewer for D32117: Update TableGen LangIntro.rst: asb.

Hi, thanks for the patch. My feedback is:

  1. Limitations of literal formats shouldn't be discussed when introducing the types.
  2. Perhaps outside the scope of this patch, but it does seem a little odd that a simple string literal can be assigned to either a string or code variable, but code fragment literals can't be assigned to strings variables (at least on the couple of months old build on LLVM I have on this machine). The existing description that a code fragment is "just a multiline string literal" seems incorrect due to this.
Tue, Apr 25, 4:55 AM
kparzysz added inline comments to D32117: Update TableGen LangIntro.rst.
Tue, Apr 25, 4:54 AM
sdardis added a comment to D32004: [mips] Rework a portion of MipsCC interface. (NFC).

Ping.

Tue, Apr 25, 4:50 AM
mkazantsev updated the diff for D32483: [EarlyCSE] Make branches unconditional if the condition is known.

Added space in the debug message.

Tue, Apr 25, 4:46 AM
mkazantsev added a dependency for D32483: [EarlyCSE] Make branches unconditional if the condition is known: D32482: [EarlyCSE] Mark the condition of assume intrinsic as true.
Tue, Apr 25, 4:44 AM
mkazantsev added a dependent revision for D32482: [EarlyCSE] Mark the condition of assume intrinsic as true: D32483: [EarlyCSE] Make branches unconditional if the condition is known.
Tue, Apr 25, 4:44 AM
mkazantsev created D32483: [EarlyCSE] Make branches unconditional if the condition is known.
Tue, Apr 25, 4:44 AM
ismail committed rL301302: Fix fuzzer.c test on platforms where CLANG_DEFAULT_CXX_STDLIB is libc++.
Fix fuzzer.c test on platforms where CLANG_DEFAULT_CXX_STDLIB is libc++
Tue, Apr 25, 4:37 AM
Meinersbur added a comment to D32431: [Polly] Added OpenCL Runtime to GPURuntime Library for GPGPU CodeGen.

I wrote a runtime with similar scope here: https://github.com/Meinersbur/prl . We were one discussing to use it for Polly as well. What's the status of that?

Tue, Apr 25, 4:31 AM · Restricted Project
avt77 abandoned D30572: Remove equal BBs from a function.
Tue, Apr 25, 4:26 AM
Meinersbur committed rL301301: [DeLICM] Use Known information when comparing Existing.Occupied and Proposed..
[DeLICM] Use Known information when comparing Existing.Occupied and Proposed.
Tue, Apr 25, 4:11 AM
Meinersbur closed D32025: [Polly][DeLICM] Use Known information when comparing Existing.Occupied and Proposed.Occupied. by committing rL301301: [DeLICM] Use Known information when comparing Existing.Occupied and Proposed..
Tue, Apr 25, 4:11 AM · Restricted Project
peter.smith added a comment to D32223: [LLD][ELF] Add InputSectionDescriptions for Orphan Sections.

The main difficulty in creating and adding the Orphan OutputSectionCommand in addOrphanSections() is that we don't have a good idea of where in the order of OutputSectionCommands to insert them. The placeOrphanSections() is intentionally run immediately after sortSections() has been called so that it can insert the OutputSectionCommands in the right place. There is a comment in Writer.c in sortSections() that says:

// The way we define an order then is:
// *  First put script sections at the start and sort the script and
//    non-script sections independently.
// *  Move each non-script section to its preferred position. We try
//    to put each section in the last position where it it can share
//    a PT_LOAD.

It may be possible to create the Orphan OutputSectionCommands early and have the comparison routine work out the ordering, although there is a comment in the same block as the one above that claims that coming up with a strict weak ordering to get the desired result is not easy.

Tue, Apr 25, 4:09 AM
lsaba added a comment to D32277: Replace slow LEA instructions in X86.

D32352 is looking at more aggressive conversion of IMUL to multiple LEA instructions.

Thanks for notifying, the patch does not contain any 3-Operand LEAs as far as I can see, the only case we need to be careful with is for 2-Operand LEA with RBP/R13/EBP as a base register, since this is determined only after RA, I am thinking it's better to let my patch fix those cases rather than preventing that patch from running on the problematic targets

Sorry for being pedantic about the naming, but for AMD architectures the 'slowlea' cases (whether it uses the ALU or AGU pipe) include scale != 1 (even for 2-ops), but it doesn't seem to be noticeably affected by RBP/R13/EBP. Hence my interest in PR32326 to try and make it easier to discriminate.

Tue, Apr 25, 4:07 AM
grimar added inline comments to D32171: [ELF] - Implemented --defsym option.
Tue, Apr 25, 4:06 AM
grimar updated the diff for D32171: [ELF] - Implemented --defsym option.
  • Addressed review comments.
Tue, Apr 25, 4:06 AM
danielmarjamaki added inline comments to D32346: [clang-tidy] New readability check for strlen argument.
Tue, Apr 25, 4:04 AM
RKSimon committed rL301300: [DAGCombiner] Use SDValue::getConstantOperandVal helper where possible. NFCI..
[DAGCombiner] Use SDValue::getConstantOperandVal helper where possible. NFCI.
Tue, Apr 25, 4:00 AM
dsanders added a comment to D32275: [globalisel][tablegen] Add several GINodeEquiv's for operators that do not require additional support..

I decided to have a look at code coverage differences before committing this. This patch made no difference to the code coverage. On closer inspection it turns out beginFunction() isn't being executed so no rules with rule predicates can match. I'll take a look at this, maybe something from the rule-predicates patch didn't make it into the final commit.

I literally just ran into this while trying to enable TableGen for ARM. I'm wondering if it's really a good idea to have beginFunction in the first place. There's a related FIXME in TargetSubtargetInfo.h around getInstructionSelector. Maybe we should have an instruction selector per MachineFunction?

Tue, Apr 25, 3:58 AM
JonasToth added a comment to D32346: [clang-tidy] New readability check for strlen argument.

#include <string.h>
void dostuff(const char *P) {

if (strncmp(P+2,"xyz",3)==0) {}

}

Iam not super familiar with C, but the intend of the function is to check the following:

P = "foxyz", P2 = "abxyz", P3 = "opxyz", ...

And if P matches this kind of string pattern.

Tue, Apr 25, 3:52 AM
RKSimon added a comment to D32376: [ValueTracking] Introduce a KnownBits struct to wrap the two APInts for computeKnownBits.

I like this but there is a lot going on in the patch and I had to stop myself adding more comments....

Tue, Apr 25, 3:52 AM
grimar retitled D32171: [ELF] - Implemented --defsym option from [ELF] - Implemented --defsym option #2 to [ELF] - Implemented --defsym option.
Tue, Apr 25, 3:50 AM
grimar abandoned D32082: [ELF] - Implemented --defsym option.

D32171 is on review instead

Tue, Apr 25, 3:50 AM