Page MenuHomePhabricator

void (Bill Wendling)
Mind Taker

Projects

User does not belong to any projects.

User Details

User Since
Dec 3 2012, 10:22 AM (342 w, 1 d)

Sent here from the planet Zvddw, Bill's mission is to conquer the world by staying at home and occasionally (read: always) playing poker.

In his spare time, he works on LLVM-related things.

Recent Activity

May 22 2019

void added inline comments to D38479: Make -mgeneral-regs-only more like GCC's.
May 22 2019, 11:29 AM

May 19 2019

void added a comment to D38479: Make -mgeneral-regs-only more like GCC's.

We (Fuchsia) would like to see this landed as well so we can start using this in our kernel.

May 19 2019, 2:46 AM

May 17 2019

void added inline comments to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.
May 17 2019, 4:27 PM · Restricted Project

May 13 2019

void added a comment to D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.

On the frontend side:

  • __builtin_constant_p is not expanded correctly in EvalConstant.cpp when used as part of codegen for -O0. This means that trivially unreachable branches are emitted (e.g. with a BR with a literal false as condition),
  • The constant evaluator has a general problem with overly pessimistic analysis when it comes to possible side effects. Still have to discuss the correct handling with Richard for this.

GCC has this to say about __builtin_constant_p's behavior:

A return of 0 does not indicate that the value is not a constant, but merely that GCC cannot prove it is a constant with the specified value of the -O option.

When I was modifying our implementation recently, I made it so that if there are no optimizations then we evaluate to 0 in the front-end before code-gen. It might be that those blocks are being emitted, but they should have something like br i1 false, label %true_bb, label %false_bb or equivalent.

Yes, that's correct. The problem is that with -O0, we never run SimplifyCFG, so those conditional branches make it straight to the backend. As discussed with Richard on IRC, the core issue is that for -O0, the IR generation for __builtin_constant_p is different from the constant folding as part of AST/EvalConstant.cpp. The latter is used by EmitIfStmt and friends to decide whether the BBs should be created in first place and currently fails evaluation for bcp.

On the backend side:

  • The is_constant intrinsic is lowered too late. The prepare-isel pass it is currently part of is run after the last SimplifyCFG pass, so the now-dead basic blocks are never pruned and therefore the asm statements survive.

Is this at -O0 or all optimization levels?

With -O0, the intrinsic is never generated and no SimplifyCFG is run either (see above).

May 13 2019, 11:15 PM · Restricted Project
void added a comment to D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.

On the frontend side:

  • __builtin_constant_p is not expanded correctly in EvalConstant.cpp when used as part of codegen for -O0. This means that trivially unreachable branches are emitted (e.g. with a BR with a literal false as condition),
  • The constant evaluator has a general problem with overly pessimistic analysis when it comes to possible side effects. Still have to discuss the correct handling with Richard for this.
May 13 2019, 5:23 PM · Restricted Project
void added a comment to D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.

It does not fix the issues on our side, but pushes them to a different place. It is still an improvement, but the problem is not solved yet.

May 13 2019, 4:45 PM · Restricted Project

May 9 2019

void committed rG6ee7f31484fc: Add ".dword" directive (authored by void).
Add ".dword" directive
May 9 2019, 2:56 PM
void committed rL360381: Add ".dword" directive.
Add ".dword" directive
May 9 2019, 2:55 PM
void closed D61719: Add ".dword" directive.
May 9 2019, 2:55 PM · Restricted Project
void updated the diff for D61719: Add ".dword" directive.

Remove errant space.

May 9 2019, 2:28 PM · Restricted Project
void added a comment to D61719: Add ".dword" directive.

Could you add a test?

May 9 2019, 1:57 PM · Restricted Project
void updated the diff for D61719: Add ".dword" directive.

Add testcase.

May 9 2019, 1:57 PM · Restricted Project
void updated the summary of D61719: Add ".dword" directive.
May 9 2019, 1:57 PM · Restricted Project

May 8 2019

void added a comment to D61719: Add ".dword" directive.

This is the doc saying dword == xword:

May 8 2019, 11:34 PM · Restricted Project
void updated subscribers of D61719: Add ".dword" directive.
May 8 2019, 11:34 PM · Restricted Project
void created D61719: Add ".dword" directive.
May 8 2019, 11:34 PM · Restricted Project

Apr 30 2019

void added a comment to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.

Right. My point is that it's equivalent to passing a pointer around, except that the pointer cannot be dereferenced anywhere except the originating function. (For the purposes of what Nick's trying to do, "originating function" includes code that's inlined into a function.) As long as IPSCCP and the like don't violate that constraint, passing the pointer around isn't going to cause semantic changes to the program. (A function that tries to jump to a block address that's not within that function is already undefined and still should be.)

Oh, you're saying that if we're inlining a function that contains an indirectbr, but we aren't inlining the indirectbr, we can allow inlining as long as we as the blockaddress still points to the original function. That's abstractly reasonable, but it probably breaks the Linux kernel's _THIS_IP_ macro (and therefore, I'm guessing, isn't what gcc does).

Apr 30 2019, 1:15 PM · Restricted Project

Apr 29 2019

void added a comment to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.

But should it?

It seems like a reasonable optimization, depending on the code structure.

That implies that a blockaddress that's propagated to other functions aren't used by anything other than arithmetic and comparison instructions.

The important thing is that it can also be returned back to the original function, either directly, or indirectly through memory.

Apr 29 2019, 1:38 PM · Restricted Project

Apr 26 2019

void added a comment to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.

that's a case I don't think can be done in C even with GNU C extensions

It can't be written explicitly in C, sure... but optimizations like IPSCCP can propagate blockaddress constants to other functions.

Apr 26 2019, 12:30 PM · Restricted Project

Apr 25 2019

void added inline comments to D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs.
Apr 25 2019, 6:20 PM · Restricted Project
void accepted D60887: [AsmPrinter] refactor to support %c w/ GlobalAddress'.

This looks like a nice cleanup. Accepting unless Peter has any objections.

Apr 25 2019, 12:59 PM · Restricted Project

Apr 22 2019

void added a comment to D60943: Delay diagnosing "n" constraint until after inlining.

Here's the motivating bug report: https://bugs.llvm.org/show_bug.cgi?id=41027

Thanks, that's illuminating. OK, if we want to support that code, then there's really not much validation we can do in the frontend for constraints that require immediate constants. Is "n" really special in this regard, or is it just the first one that we've encountered?

Apr 22 2019, 9:25 PM · Restricted Project
void added a child revision for D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate: D60943: Delay diagnosing "n" constraint until after inlining.
Apr 22 2019, 5:32 PM · Restricted Project
void added a parent revision for D60943: Delay diagnosing "n" constraint until after inlining: D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.
Apr 22 2019, 5:32 PM · Restricted Project
void added a comment to D60943: Delay diagnosing "n" constraint until after inlining.

Here's the motivating bug report: https://bugs.llvm.org/show_bug.cgi?id=41027

Apr 22 2019, 5:31 PM · Restricted Project
void added a comment to D60943: Delay diagnosing "n" constraint until after inlining.

I'm in the process of testing this, but feedback will take a bit.

On the more meaty parts of this change, I think further iterations will be necessary in-tree to extend this to the other constraints.

Apr 22 2019, 1:45 PM · Restricted Project

Apr 21 2019

void added a comment to D60943: Delay diagnosing "n" constraint until after inlining.

This relies upon D60943.

Apr 21 2019, 6:23 PM · Restricted Project
void added reviewers for D60943: Delay diagnosing "n" constraint until after inlining: joerg, eli.friedman, rsmith.
Apr 21 2019, 6:23 PM · Restricted Project
void updated the diff for D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.

Add testcase.

Apr 21 2019, 5:20 PM · Restricted Project
void updated the diff for D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.

Fix testcase to get the correct error message.

Apr 21 2019, 3:21 AM · Restricted Project
void updated the diff for D60943: Delay diagnosing "n" constraint until after inlining.

Fix test. Use "e" instead of "n".

Apr 21 2019, 3:12 AM · Restricted Project
void added reviewers for D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate: joerg, mgorny.
Apr 21 2019, 3:12 AM · Restricted Project
void updated the diff for D60943: Delay diagnosing "n" constraint until after inlining.

Put constraint string check in the correct place.

Apr 21 2019, 2:31 AM · Restricted Project
void created D60942: Emit diagnostic if inline asm "n" constraint isn't an immediate.
Apr 21 2019, 1:39 AM · Restricted Project
void created D60943: Delay diagnosing "n" constraint until after inlining.
Apr 21 2019, 1:39 AM · Restricted Project

Apr 15 2019

void added a comment to D56571: [RFC prototype] Implementation of asm-goto support in clang.

I think things don't work right unless you disable the integrated assembler. I'm not sure why though.

Apr 15 2019, 5:23 PM
void added a comment to D56571: [RFC prototype] Implementation of asm-goto support in clang.
; ModuleID = 'arch_static_branch.bc'
source_filename = "arch/x86/entry/vsyscall/vsyscall_64.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-grtev4-linux-gnu"
Apr 15 2019, 12:18 PM

Apr 13 2019

void committed rG191f1487b63b: [X86] Use PC-relative mode for the kernel code model (authored by void).
[X86] Use PC-relative mode for the kernel code model
Apr 13 2019, 2:43 PM
void committed rL358343: [X86] Use PC-relative mode for the kernel code model.
[X86] Use PC-relative mode for the kernel code model
Apr 13 2019, 2:42 PM
void closed D60643: [X86] Use PC-relative mode for the kernel code model.
Apr 13 2019, 2:42 PM · Restricted Project
void added inline comments to D60643: [X86] Use PC-relative mode for the kernel code model.
Apr 13 2019, 2:02 AM · Restricted Project

Apr 12 2019

void created D60643: [X86] Use PC-relative mode for the kernel code model.
Apr 12 2019, 6:18 PM · Restricted Project

Mar 1 2019

void added a comment to D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890).

My opinion doesn't carry as much weight as others who are more familiar with the front-end code, but LGTM.

Mar 1 2019, 11:25 AM · Restricted Project

Feb 26 2019

void committed rG01706bda5b60: Output ELF files after ThinLTO is run. (authored by void).
Output ELF files after ThinLTO is run.
Feb 26 2019, 11:31 AM
void committed rL354917: Output ELF files after ThinLTO is run..
Output ELF files after ThinLTO is run.
Feb 26 2019, 11:31 AM
void committed rLLD354917: Output ELF files after ThinLTO is run..
Output ELF files after ThinLTO is run.
Feb 26 2019, 11:31 AM
void closed D56046: Output ELF files after ThinLTO is run..
Feb 26 2019, 11:31 AM · Restricted Project
void updated the diff for D56046: Output ELF files after ThinLTO is run..

Move checks over to the main LL file, not Input file.

Feb 26 2019, 11:31 AM · Restricted Project

Feb 24 2019

void added a comment to D56046: Output ELF files after ThinLTO is run..

@pcc: Ping? :)

Feb 24 2019, 8:39 PM · Restricted Project

Feb 20 2019

void added a comment to D56046: Output ELF files after ThinLTO is run..

Another ping. Anyone? :-)

Feb 20 2019, 2:27 PM · Restricted Project

Feb 14 2019

void added a comment to D56046: Output ELF files after ThinLTO is run..

Friendly ping. :-)

Feb 14 2019, 12:56 PM · Restricted Project

Feb 13 2019

void added a comment to D56508: [llvm-ar] Flatten thin archives..

That should be fixed by D57842, which I'm planning to submit soon -- I want to poke a few more test cases first.

Feb 13 2019, 2:25 PM · Restricted Project
Herald added a project to D56508: [llvm-ar] Flatten thin archives.: Restricted Project.

This fails if the archives are nested. E.g.:

Feb 13 2019, 2:11 PM · Restricted Project

Feb 11 2019

Herald added a project to D56046: Output ELF files after ThinLTO is run.: Restricted Project.

Ping? :-)

Feb 11 2019, 12:42 PM · Restricted Project

Feb 8 2019

void added a comment to D57267: [AST] Factor out the logic of the various Expr::Ignore*.

@void @rnk Perhaps you can comment on this: currently Expr::IgnoreImpCasts skips FullExprs, but Expr::IgnoreParenImpCasts only skips (via IgnoreParens) ConstantExprs. Is there any reason for this inconsistency ? I would like to add FullExpr to the nodes skipped by IgnoreParenImpCasts for consistency but I am worried about unexpected issues even though all tests pass.

Feb 8 2019, 12:25 PM · Restricted Project, Restricted Project

Jan 29 2019

void accepted D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects..

LGTM, but may want to wait for other reviewers.

Jan 29 2019, 5:22 PM · Restricted Project

Jan 26 2019

void committed rL352307: Remove Expr sugar decorating the CXXUuidofExpr node..
Remove Expr sugar decorating the CXXUuidofExpr node.
Jan 26 2019, 11:25 PM
void committed rC352307: Remove Expr sugar decorating the CXXUuidofExpr node..
Remove Expr sugar decorating the CXXUuidofExpr node.
Jan 26 2019, 11:24 PM
void closed D57114: Remove Expr sugar decorating the CXXUuidofExpr node..
Jan 26 2019, 11:24 PM

Jan 23 2019

void updated the diff for D57114: Remove Expr sugar decorating the CXXUuidofExpr node..

Move test to proper place and add "-verify" flag.

Jan 23 2019, 2:33 PM
void added inline comments to D57114: Remove Expr sugar decorating the CXXUuidofExpr node..
Jan 23 2019, 2:33 PM
void created D57114: Remove Expr sugar decorating the CXXUuidofExpr node..
Jan 23 2019, 12:13 PM

Jan 17 2019

void added a comment to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.

I encountered another ICE here: https://github.com/ClangBuiltLinux/linux/issues/320#issuecomment-455456506

Jan 17 2019, 11:49 PM · Restricted Project

Jan 16 2019

void added a comment to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.

There appears to be an ICE with this patch: https://github.com/ClangBuiltLinux/linux/issues/6#issuecomment-454675962

Jan 16 2019, 5:42 PM · Restricted Project

Jan 15 2019

void added a comment to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.

I like these changes. Could you add the documentation for the new instruction?

Jan 15 2019, 12:43 PM · Restricted Project

Jan 14 2019

void added inline comments to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.
Jan 14 2019, 1:10 PM · Restricted Project
void added inline comments to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.
Jan 14 2019, 12:59 PM · Restricted Project

Jan 11 2019

void added a comment to D55616: Emit ASM input in a constant context.

Apologies - the value seems to indeed overflow, but I'm still very confused how this was affected by this change.

Jan 11 2019, 5:58 PM · Restricted Project

Jan 10 2019

void added inline comments to D53765: [RFC prototype] Implementation of asm-goto support in LLVM.
Jan 10 2019, 5:48 PM · Restricted Project

Jan 4 2019

void added inline comments to D56046: Output ELF files after ThinLTO is run..
Jan 4 2019, 3:30 PM · Restricted Project
void updated the diff for D56046: Output ELF files after ThinLTO is run..

Implement Rui's suggestion for how to output the ELF files.

Jan 4 2019, 3:28 PM · Restricted Project

Jan 2 2019

void updated the diff for D56046: Output ELF files after ThinLTO is run..

Add check that the object file is still valid after using "obj-path".

Jan 2 2019, 5:33 PM · Restricted Project

Dec 31 2018

void added a comment to D56046: Output ELF files after ThinLTO is run..

The filenames of Task=0 and different. Is this preferable?

t.o t2.o were compiled with -module-summary

ld.lld --plugin-opt=obj-path=t3.o -shared t.o t2.o => creates t2.o0 t2.o1 t2.o2

gold --plugin=~/llvm/Release/lib/LLVMgold.so --plugin-opt=thinlto --plugin-opt=obj-path=t3.o -shared t.o t2.o => creates t2.o t2.o1 t2.o2

Dec 31 2018, 10:08 PM · Restricted Project
void updated the diff for D56046: Output ELF files after ThinLTO is run..

Don't append "0" to the object filename.

Dec 31 2018, 10:08 PM · Restricted Project
void updated the diff for D56046: Output ELF files after ThinLTO is run..

Add testcase.

Dec 31 2018, 10:05 PM · Restricted Project

Dec 27 2018

void added a comment to D56046: Output ELF files after ThinLTO is run..

Replicating the gold behavior sounds fine, but does this patch work? This change seems to leave Buf uninitialized while Buf will be used later. Also, in gold LTO plugin, -obj-path overwrites -save-temps, but this change doesn't seem to replicate that behavior. Could you add tests?

Dec 27 2018, 2:38 PM · Restricted Project
void added reviewers for D56046: Output ELF files after ThinLTO is run.: ruiu, MaskRay.
Dec 27 2018, 12:54 PM · Restricted Project
void added a comment to D56046: Output ELF files after ThinLTO is run..

Ping? :-)

Dec 27 2018, 12:54 PM · Restricted Project

Dec 21 2018

void created D56046: Output ELF files after ThinLTO is run..
Dec 21 2018, 11:33 PM · Restricted Project

Dec 18 2018

void added a comment to D55616: Emit ASM input in a constant context.

Looks like it's broken by this patch

clang: /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/AST/ExprConstant.cpp:11055: llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext &, SmallVectorImpl<clang::PartialDiagnosticAt> *) const: Assertion `Result && "Could not evaluate expression"' failed.

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/9281/steps/check-clang%20msan/logs/stdio

Dec 18 2018, 9:02 PM · Restricted Project
void committed rL349604: Use "EvaluateAsRValue" instead of as a known int, because if it's not a known.
Use "EvaluateAsRValue" instead of as a known int, because if it's not a known
Dec 18 2018, 8:59 PM
void committed rC349604: Use "EvaluateAsRValue" instead of as a known int, because if it's not a known.
Use "EvaluateAsRValue" instead of as a known int, because if it's not a known
Dec 18 2018, 8:59 PM
void committed rL349603: Revert accidentally included code..
Revert accidentally included code.
Dec 18 2018, 8:40 PM
void committed rC349603: Revert accidentally included code..
Revert accidentally included code.
Dec 18 2018, 8:40 PM
void committed rL349561: Emit ASM input in a constant context.
Emit ASM input in a constant context
Dec 18 2018, 2:58 PM
void committed rC349561: Emit ASM input in a constant context.
Emit ASM input in a constant context
Dec 18 2018, 2:57 PM
void closed D55616: Emit ASM input in a constant context.
Dec 18 2018, 2:57 PM · Restricted Project
void updated the diff for D55616: Emit ASM input in a constant context.

Fix how prefixes are used in the testcase.

Dec 18 2018, 2:57 PM · Restricted Project
void added inline comments to D55616: Emit ASM input in a constant context.
Dec 18 2018, 2:57 PM · Restricted Project

Dec 17 2018

void updated the diff for D55616: Emit ASM input in a constant context.

Reduce duplication by using multiple check prefixes.

Dec 17 2018, 8:10 PM · Restricted Project
void added inline comments to D55616: Emit ASM input in a constant context.
Dec 17 2018, 3:09 PM · Restricted Project
void added a comment to D55616: Emit ASM input in a constant context.

Ping? :-)

Dec 17 2018, 1:08 PM · Restricted Project

Dec 13 2018

void added a comment to D55616: Emit ASM input in a constant context.

This seems like a good start, but not complete.

"n" and "i" both should require that their argument is a constant expression. For "n", it actually must be an immediate constant integer, so setRequiresImmediate() should be used there. For "i", you may use an lvalue constant as well. The way we seem to indicate that, today, is with if (!Info.allowsRegister() && !Info.allowsMemory()). However the code in that block does not today *require* that the evaluation as a constant succeed...and it should.

It should also only require that the result is an integer when requiresImmediateConstant().

Additionally, the Sema code needs to have the same conditions as the CodeGen code for when a constant expression is required, but it doesn't. (before or after your change).

Dec 13 2018, 4:12 PM · Restricted Project
void updated the diff for D55616: Emit ASM input in a constant context.

The n constriant *requires* a known integer. Therefore, enforce this in both
Sema and CodeGen by setting the "requires immediate" flag and evaluating to a
known integer instead of random "int"..

Dec 13 2018, 4:11 PM · Restricted Project

Dec 12 2018

void added a comment to D55616: Emit ASM input in a constant context.

Addressed Eli's comment.

Dec 12 2018, 2:43 PM · Restricted Project
void updated the diff for D55616: Emit ASM input in a constant context.

Don't look at the optimization level here.

Dec 12 2018, 2:42 PM · Restricted Project
void added a comment to D54355: Use is.constant intrinsic for __builtin_constant_p.

The issue is that "n" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of __builtin_constant_p shouldn't be delayed (it being compiled at -O0). I suspect that this issue will also happen with the "i" constraint.

Indeed. We _do_ actually already know that in clang too, however -- clang already knows that "n" requires an immediate, and does constant expression evaluation for it, e.g. to expand constexpr functions. Grep for "requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p correctly, though.

(Note, as a workaround, you could use | instead of ||, or put the expression as the value of an enumeration constant).

Dec 12 2018, 2:12 PM
void added a comment to D55616: Emit ASM input in a constant context.

As a side note, the number of ways to evaluate a constant expression are legion in clang. They should really be unified...

Dec 12 2018, 2:00 PM · Restricted Project
void created D55616: Emit ASM input in a constant context.
Dec 12 2018, 2:00 PM · Restricted Project
void updated subscribers of D55616: Emit ASM input in a constant context.
Dec 12 2018, 2:00 PM · Restricted Project