Page MenuHomePhabricator

bjope (Bjorn Pettersson)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 26 2016, 7:58 AM (212 w, 6 d)

Recent Activity

Fri, Oct 23

bjope added a comment to rG529ac33197f6: [libc++abi] Fix the standalone build after the __config_site change.

Our "master integration" also stopped after this commit, and we hit the LIBCXXABI_LIBCXX_INCLUDES= is not a valid directory. error. We've never provided any LIBCXXABI_LIBCXX_INCLUDES so I'm not sure what I should point it to.

Fri, Oct 23, 5:21 AM

Wed, Oct 21

bjope added a comment to D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

As I see it this solution focuses on the problem seen in LoopUnroll. So I agree, it is conservative and simple so if it solves the PR I think it is just fine.

Wed, Oct 21, 3:36 AM · Restricted Project

Mon, Oct 19

bjope added inline comments to D89640: [Sanitizers] Remove OpenBSD support.
Mon, Oct 19, 11:01 AM · Restricted Project

Wed, Oct 7

bjope added a comment to D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746).

So: skip the call to RemoveRedundantDbgInstrs. There's surprisingly little fallout from this, and most of it can be addressed by doing RemoveRedundantDbgInstrs later.

As an alternative, would it make sense to have a dedicated pass to eliminate redundant debug instructions, that's run late in the pipeline? not sure if running it once, but for every function will be better or worse in the end. It should be easy to skip for functions without debug info though.

Wed, Oct 7, 2:18 AM · Restricted Project

Mon, Oct 5

bjope added a comment to D86203: [GlobalISel][TableGen] Add handling of unannotated dst pattern ops.

One thing that I've not understood is how GlobalISel is different from the legacy ISel here. Apparently it isn't necessary to annotate things with the legacy ISel today? So is legacy ISel doing the same thing that this patch suggests to do also for GlobalISel, or why do we suddenly need to update all patterns now (I mean, somehow it has worked fine in the past, right)?

Mon, Oct 5, 5:33 AM · Restricted Project
bjope added inline comments to D48803: Place the BlockAddress type in the program address space.
Mon, Oct 5, 4:42 AM · Restricted Project

Fri, Oct 2

bjope added a comment to D88454: [CMake] Use -isystem flag to access libc++ headers.

I think I figured it out, D88756 should address the issue.

Fri, Oct 2, 3:16 PM · Restricted Project, Restricted Project
bjope added a comment to D88454: [CMake] Use -isystem flag to access libc++ headers.

Looking at build.ninja we used to get

Fri, Oct 2, 12:45 PM · Restricted Project, Restricted Project
bjope accepted D87494: Improve LSR debug-info.

I've checked the late "better safe than sorry type checks" and they LGTM.

Fri, Oct 2, 6:36 AM · Restricted Project, debug-info
bjope added a comment to D88454: [CMake] Use -isystem flag to access libc++ headers.

Hi!
I don't understand why, but with this commit I start seeing several warnings like this when compiling builtins:

/data/repo/master/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h:45:7: error: '__sanitizer::FlagHandler<bool>' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor]
class FlagHandler : public FlagHandlerBase {
      ^
/data/repo/master/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h:71:13: note: in instantiation of template class '__sanitizer::FlagHandler<bool>' requested here
inline bool FlagHandler<bool>::Parse(const char *value) {
            ^

That's a bit surprising. What's your build configuration? Do you use runtimes build? I haven't managed to reproduce it locally so far.

Fri, Oct 2, 4:52 AM · Restricted Project, Restricted Project

Tue, Sep 29

bjope accepted D87494: Improve LSR debug-info.

One more nit related to making computeConstantDifference public , but I'm happy with the fixup related to making the test case more target-agnostic.

Tue, Sep 29, 4:39 AM · Restricted Project, debug-info

Mon, Sep 28

bjope added inline comments to D87494: Improve LSR debug-info.
Mon, Sep 28, 12:58 AM · Restricted Project, debug-info
bjope added inline comments to D87494: Improve LSR debug-info.
Mon, Sep 28, 12:56 AM · Restricted Project, debug-info

Sep 18 2020

bjope added a comment to D83573: [libunwind] Support for leaf function unwinding..

Sorry for the trouble for now the patch is reverted.
I you agre I will resubmit it with ; REQUIRES: x86_64-linux and then figure out the rest of the targets later.

Thanks,
Daniel

Sep 18 2020, 4:17 AM · Restricted Project, Restricted Project
bjope added a comment to D86215: [TableGen][GlobalISel] Fix handling of zero_reg.

I landed this on behalf of Gabriel.

Sep 18 2020, 2:03 AM · Restricted Project
bjope committed rGc10200536f2e: [TableGen][GlobalISel] Fix handling of zero_reg (authored by ehjogab).
[TableGen][GlobalISel] Fix handling of zero_reg
Sep 18 2020, 2:02 AM
bjope closed D86215: [TableGen][GlobalISel] Fix handling of zero_reg.
Sep 18 2020, 2:02 AM · Restricted Project

Sep 17 2020

bjope updated subscribers of D83573: [libunwind] Support for leaf function unwinding..
Sep 17 2020, 5:46 AM · Restricted Project, Restricted Project
bjope added a comment to D83573: [libunwind] Support for leaf function unwinding..

Having some problems after this patch, when building the runtimes for i386, on a 64-bit RHEL7 server, and then running the added libunwind test cases on the same host.
No expert here, and maybe that isn't a valid scenario, but building/running tests like that has worked pretty good before.

Sep 17 2020, 5:45 AM · Restricted Project, Restricted Project

Sep 16 2020

bjope added inline comments to D82678: [CGP] Set debug locations when optimizing phi types.
Sep 16 2020, 9:01 AM · debug-info, Restricted Project
bjope added a comment to D82678: [CGP] Set debug locations when optimizing phi types.

Just want to clarify that my questions regarding "what about debug info" in the earlier patch introducing these phi-type-rewrites actually was about dbg.value rather than the debugloc:s. I kind of wanted to have test cases verifying that the the presence of dbg.value uses didn't impact the optimization (-g invariance). But also to see that the dbg.value uses were handled somehow (either being salvaged in some way or being properly invalidated).

Sep 16 2020, 8:43 AM · debug-info, Restricted Project

Sep 15 2020

bjope committed rGaa8be5aeead7: [Scalarizer] Avoid changing name of non-instructions (authored by bjope).
[Scalarizer] Avoid changing name of non-instructions
Sep 15 2020, 5:17 AM
bjope closed D87685: [Scalarizer] Avoid changing name of non-instructions.
Sep 15 2020, 5:16 AM · Restricted Project
bjope requested review of D87685: [Scalarizer] Avoid changing name of non-instructions.
Sep 15 2020, 4:41 AM · Restricted Project

Sep 4 2020

bjope added inline comments to D85818: [UnifyFunctionExitNodes] Fix Modified status for unreachable blocks.
Sep 4 2020, 9:42 AM · Restricted Project
bjope accepted D87078: [UnifyFunctionExitNodes] Remove unused getters, NFC.

Just hoping that those unused functions aren't used by some OOT project. But considering that the API is totally untested (and the unwind part isn't even visible any longer) I think this cleanup is motivated.

Sep 4 2020, 9:17 AM · Restricted Project

Sep 2 2020

bjope added a comment to D86839: [CMake] Remove -Wl,-allow-shlib-undefined which was added in rL221530.

Things seem to link again for us if we set LLVM_ENABLE_TERMINFO=OFF (as a workaround to avoid dependency to libtinfo), but I guess we might lose some color output or something by doing that.

Sep 2 2020, 10:25 AM · Restricted Project
bjope added a comment to D86839: [CMake] Remove -Wl,-allow-shlib-undefined which was added in rL221530.

So are we perhaps missing tinfo (and ncurses) in /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64?
I doubt that we should pick up libtinfo from /usr/lib64 here, even if we do not really need to crosscompile llvm-tblgen (tblgen should only execute on the current host anyway).

Sep 2 2020, 10:13 AM · Restricted Project

Sep 1 2020

bjope added inline comments to D85818: [UnifyFunctionExitNodes] Fix Modified status for unreachable blocks.
Sep 1 2020, 6:30 AM · Restricted Project
bjope added a comment to D86586: [TableGen] Support tied operands that are both source operands.

I don't really understand the problem situation. Why are you having two source operands if they need to be the same? Can't you ensure that by only having one source operand?

Sep 1 2020, 2:00 AM · Restricted Project

Aug 26 2020

bjope added inline comments to D86215: [TableGen][GlobalISel] Fix handling of zero_reg.
Aug 26 2020, 9:03 AM · Restricted Project
bjope committed rG5e23dc5b4702: [GlobalISel] Fix and tidy up documentation for ValueMapping class (NFC) (authored by ehjogab).
[GlobalISel] Fix and tidy up documentation for ValueMapping class (NFC)
Aug 26 2020, 3:11 AM
bjope closed D86201: [GlobalISel] Fix and tidy up documentation for ValueMapping class (NFC).
Aug 26 2020, 3:11 AM · Restricted Project
bjope committed rGb2b9af5a1397: [TableGen][GlobalISel] Fix tblgen optimization bug (authored by ehjogab).
[TableGen][GlobalISel] Fix tblgen optimization bug
Aug 26 2020, 3:11 AM
bjope closed D86199: [TableGen][GlobalISel] Fix tblgen optimization bug.
Aug 26 2020, 3:11 AM · Restricted Project

Aug 25 2020

bjope added a comment to D86201: [GlobalISel] Fix and tidy up documentation for ValueMapping class (NFC).

Should I land this patch for you (including a fix for the long lines)?

Aug 25 2020, 1:30 AM · Restricted Project
bjope added a comment to D86199: [TableGen][GlobalISel] Fix tblgen optimization bug.

@ehjogab : Should I help out landing this patch?

Aug 25 2020, 1:29 AM · Restricted Project

Aug 24 2020

bjope committed rGfce44ff5da4e: [Scalarizer] Avoid updating the name of globals (authored by bjope).
[Scalarizer] Avoid updating the name of globals
Aug 24 2020, 12:56 PM
bjope closed D86472: [Scalarizer] Avoid updating the name of globals.
Aug 24 2020, 12:56 PM · Restricted Project
bjope added a comment to D86472: [Scalarizer] Avoid updating the name of globals.

@bjope thanks! bugreport kinda skimmed my attention span, i was going to re-look today-ish.
I think this looks fine. The another alternative would be to only do that if the value-to-be-renamed is an instruction,
but then i'm not sure it's an improvement if we rename something that we reused.

Aug 24 2020, 12:24 PM · Restricted Project
bjope added inline comments to D86215: [TableGen][GlobalISel] Fix handling of zero_reg.
Aug 24 2020, 10:52 AM · Restricted Project
bjope added a comment to D83101: [Scalarizer] ExtractElement handling w/ constant extract index.

Proposed fix for the problem described by @materi : https://reviews.llvm.org/D86472

Aug 24 2020, 10:18 AM · Restricted Project
bjope requested review of D86472: [Scalarizer] Avoid updating the name of globals.
Aug 24 2020, 10:17 AM · Restricted Project
bjope accepted D86438: [SDAG] Convert FSHL <--> FSHR if the target only supports one of them.

LGTM (except the earlier nit about the comments with different X and Y values on left and right hand side)

Aug 24 2020, 9:00 AM · Restricted Project
bjope added inline comments to D86438: [SDAG] Convert FSHL <--> FSHR if the target only supports one of them.
Aug 24 2020, 8:56 AM · Restricted Project
bjope added a comment to D60413: [BDCE] SExt -> ZExt when no sign bits is used and instruction has multiple uses.

I've submitted a PR, https://bugs.llvm.org/show_bug.cgi?id=47296, regarding the debuginfo problem.

Aug 24 2020, 6:08 AM · Restricted Project
bjope committed rG7a4e26adc8c2: [SelectionDAG] Fix miscompile bug in expandFunnelShift (authored by bjope).
[SelectionDAG] Fix miscompile bug in expandFunnelShift
Aug 24 2020, 12:53 AM
bjope closed D86430: [SelectionDAG] Fix miscompile bug in expandFunnelShift.
Aug 24 2020, 12:53 AM · Restricted Project
bjope added a comment to D86430: [SelectionDAG] Fix miscompile bug in expandFunnelShift.

It could be possible to add back the transform, given that logic
is added to check that (Z % BW) can't be zero. Since there were
no test cases proving that such a transform actually would be useful
I decided to simply remove the faulty code in this patch.

I don't understand this. The AMDGPU test changes clearly show that the transform is useful. AMDGPU has an alignbit instruction which is fshr, but no instruction for fshl.

Oh I see, I guess you mean that the check for non-zero Z didn't help any of these test cases, because none of them use a shift amount that is known to be non-zero.

OK then. I can try to fix the AMDGPU regressions as a follow up, maybe with a target-specific lowering.

Aug 24 2020, 12:49 AM · Restricted Project
bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 24 2020, 12:31 AM · Restricted Project
bjope requested review of D86430: [SelectionDAG] Fix miscompile bug in expandFunnelShift.
Aug 24 2020, 12:30 AM · Restricted Project

Aug 23 2020

bjope abandoned D85969: [SCEV] Model (xor (shl x, C), (-1 << C)) as (shl (xor x, -1), C).

Haven't seen any gains from this patch, after having rebased to https://reviews.llvm.org/rGec06b381304140b2553cfdfae5a063f39c5c59ff .
The test I ran included bunch of application code, compiled with -O3, for our OOT target.

Aug 23 2020, 1:37 PM · Restricted Project

Aug 21 2020

bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 21 2020, 1:58 PM · Restricted Project
bjope added inline comments to D65471: [AMDGPU] Reserve all AGPRs on targets which do not have them.
Aug 21 2020, 12:17 PM · Restricted Project
bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 21 2020, 10:31 AM · Restricted Project
bjope added inline comments to D65471: [AMDGPU] Reserve all AGPRs on targets which do not have them.
Aug 21 2020, 6:07 AM · Restricted Project
bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 21 2020, 5:43 AM · Restricted Project
bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 21 2020, 5:33 AM · Restricted Project
bjope added inline comments to D77152: [SelectionDAG] Better legalization for FSHL and FSHR.
Aug 21 2020, 5:10 AM · Restricted Project

Aug 20 2020

bjope added a comment to D86243: [InstCombine] canonicalize 'not' ops before logical shifts.

Some SCEV/pipeline tests?

Maybe from https://reviews.llvm.org/D85969#change-8DkHYel6Y1JK ?

Aug 20 2020, 6:59 AM · Restricted Project
bjope added a comment to D86243: [InstCombine] canonicalize 'not' ops before logical shifts.

I've verified that this resolves https://bugs.llvm.org/show_bug.cgi?id=47136 in the sense that our downstream benchmark result is back to the original result for the tests that had a regression. And without any new regressions. So that is perfect!

Aug 20 2020, 6:55 AM · Restricted Project
bjope added inline comments to D86215: [TableGen][GlobalISel] Fix handling of zero_reg.
Aug 20 2020, 6:31 AM · Restricted Project
bjope committed rGff107eed1546: [AArch64] Update a code comment incorrectly referring to zero_reg. NFC (authored by bjope).
[AArch64] Update a code comment incorrectly referring to zero_reg. NFC
Aug 20 2020, 5:38 AM
bjope added a comment to D86272: [DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian.

I do not have any upstream test case for this, but got failures in our downstream testing.

Aug 20 2020, 2:53 AM · Restricted Project
bjope committed rGb43235a76c23: [DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian (authored by bjope).
[DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian
Aug 20 2020, 2:49 AM
bjope closed D86272: [DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian.
Aug 20 2020, 2:49 AM · Restricted Project
bjope requested review of D86272: [DebugInfo] Fix DwarfExpression::addConstantFP for float on big-endian.
Aug 20 2020, 2:43 AM · Restricted Project
bjope added inline comments to D86201: [GlobalISel] Fix and tidy up documentation for ValueMapping class (NFC).
Aug 20 2020, 1:02 AM · Restricted Project

Aug 19 2020

bjope added inline comments to D86215: [TableGen][GlobalISel] Fix handling of zero_reg.
Aug 19 2020, 10:17 AM · Restricted Project
bjope committed rG54105d635d18: [GlobalISel] Untabify InstructionSelectorImpl.h. NFC (authored by bjope).
[GlobalISel] Untabify InstructionSelectorImpl.h. NFC
Aug 19 2020, 3:01 AM
bjope added a comment to D60413: [BDCE] SExt -> ZExt when no sign bits is used and instruction has multiple uses.

One minor problem with implementing a reverse transform for targets that prefer sext over zext, e.g. in CodeGenPrepare, is that it will be hard to restore the debug info (considering that we do the right thing here an invalidate metadata uses).

Aug 19 2020, 1:49 AM · Restricted Project

Aug 18 2020

bjope added a comment to D60413: [BDCE] SExt -> ZExt when no sign bits is used and instruction has multiple uses.

Having some problems with this patch downstream:

  1. Patch does not update any dbg.value that might refer to the value that now is zext:ed instead of sext:ed. So debugging experience might become weird. It might be possible to rewrite debug uses, using some nifty debug expression to express a trunc+sext of the value. But in my case it was an <128 x i40> vector, and we got no dwarf expressions to express a vector sext afaik. Instead I guess any debug uses should be invalidated.

Isn't that an issue for any of the other places that do such a transform, among other things?
This very transform also exists in InstCombine and CVP.

Aug 18 2020, 9:12 AM · Restricted Project
bjope added a comment to D60413: [BDCE] SExt -> ZExt when no sign bits is used and instruction has multiple uses.

Having some problems with this patch downstream:

Aug 18 2020, 8:42 AM · Restricted Project

Aug 15 2020

bjope added a comment to D85969: [SCEV] Model (xor (shl x, C), (-1 << C)) as (shl (xor x, -1), C).

I don't have much experience with SCEV either, but I think your draft change for instcombine made sense:
https://bugs.llvm.org/show_bug.cgi?id=47136#c8

As hinted there, that's the same direction as:
D32255 / rG23bd33c6acc4

So I'm not sure if we would consider this a worthwhile improvement independently and still make a change to SCEV or only change instcombine (or if there's some reason to *not* change instcombine, I'd like to understand that).

Aug 15 2020, 2:32 PM · Restricted Project

Aug 14 2020

bjope added inline comments to D85971: [IndVarSimplify] Fix Modified status for removal of overflow intrinsics.
Aug 14 2020, 10:31 AM · Restricted Project
bjope added inline comments to D81647: MIR Statepoint refactoring. Part 3: Spill GC Ptr regs..
Aug 14 2020, 8:12 AM · Restricted Project
bjope committed rGb395d67a886c: [Orc] Fix werror for unused variable in noasserts build (authored by bjope).
[Orc] Fix werror for unused variable in noasserts build
Aug 14 2020, 6:59 AM
bjope requested review of D85969: [SCEV] Model (xor (shl x, C), (-1 << C)) as (shl (xor x, -1), C).
Aug 14 2020, 5:41 AM · Restricted Project

Aug 13 2020

bjope added a comment to D80525: [clangd] Fix crash-bug in preamble indexing when using modules..

Looks like it's an issue with not clearing the module cache. I'll revert this change to fix the build, then re-submit with proper fix.

Aug 13 2020, 9:44 AM · Restricted Project
bjope committed rG11446b02c7ec: [VectorCombine] Fix for non-zero addrspace when creating vector load from… (authored by bjope).
[VectorCombine] Fix for non-zero addrspace when creating vector load from…
Aug 13 2020, 9:27 AM
bjope closed D85912: [VectorCombine] Fix for non-zero addrspace when creating vector load from scalar load.
Aug 13 2020, 9:26 AM · Restricted Project
bjope requested review of D85912: [VectorCombine] Fix for non-zero addrspace when creating vector load from scalar load.
Aug 13 2020, 8:44 AM · Restricted Project
bjope added a comment to D80525: [clangd] Fix crash-bug in preamble indexing when using modules..

Might be that it is some other commit that cause the problem. Looking a bit closer at the jenkins history it seems like the tests passed once after this patch was added. But the next time it failed.

Aug 13 2020, 7:48 AM · Restricted Project
bjope added a comment to D80525: [clangd] Fix crash-bug in preamble indexing when using modules..

This is quite strange. I can't reproduce this with gcc 7.5.0, 9.3.0 or 10.0.1. Which version are you using?

There are bugs around clangd + modules + diagnostics. I'm fixing one in https://reviews.llvm.org/D85753 (not in it's final form yet), but it doesn't seem to be the same issue (can't be sure, but I think this is using the right SourceManager). This test is not supposed to generate any diagnostics, so that's the really surprising bit. Are you building from clean HEAD, or do you have any local modifications that could generate a extra diagnostics in this test?

The other option is that the failure is caused by module_cache_path. This test, unlike other clangd tests, needs real file system (to write the .pcm file and then read it back) and perhaps that doesn't work in your jenkins setup.

Are there any special options or things unique to your setup I could try out to reproduce this?

Aug 13 2020, 7:40 AM · Restricted Project

Aug 12 2020

bjope added a comment to D85446: [InstCombine] Add vector support to mul(add(x,c),negpow2) -> mul(sub(-c,x),pow2) folds.

(Just some heads up, in case someone else got the same problem...)

I've noticed some regressions downstream after this patch. Haven't debugged/reduced it completely yet, but at least in one case I suspect that the problem actually might be in load-store-vectorizer.

Used to get something like this before load-store-vectorizer:

<...>

The old input to load-store-vectorizer resulted in two <8 x i16> loads, but the new input gives one <8 x i16 load, two <2 x i16> loads and one <4 x i16> load. So for some reason LSV fails to detect that the loads are consecutive given the new IR. At least that is my current theory.

Please can you file a bug with reproducer?

Yes, I'm working on that (although progress has been a bit slow at the moment).

Aug 12 2020, 8:18 AM · Restricted Project
bjope added a comment to D85446: [InstCombine] Add vector support to mul(add(x,c),negpow2) -> mul(sub(-c,x),pow2) folds.

(Just some heads up, in case someone else got the same problem...)

I've noticed some regressions downstream after this patch. Haven't debugged/reduced it completely yet, but at least in one case I suspect that the problem actually might be in load-store-vectorizer.

Used to get something like this before load-store-vectorizer:

<...>

The old input to load-store-vectorizer resulted in two <8 x i16> loads, but the new input gives one <8 x i16 load, two <2 x i16> loads and one <4 x i16> load. So for some reason LSV fails to detect that the loads are consecutive given the new IR. At least that is my current theory.

Please can you file a bug with reproducer?

Aug 12 2020, 3:22 AM · Restricted Project

Aug 9 2020

bjope added a comment to D85446: [InstCombine] Add vector support to mul(add(x,c),negpow2) -> mul(sub(-c,x),pow2) folds.

(Just some heads up, in case someone else got the same problem...)

Aug 9 2020, 3:31 PM · Restricted Project

Aug 7 2020

bjope accepted D85157: [Sema] Add casting check for fixed to fixed point conversions.

LGTM!

Aug 7 2020, 4:37 AM · Restricted Project

Aug 6 2020

bjope added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Aug 6 2020, 11:39 PM · Restricted Project
bjope added inline comments to rGe8760bb9a8a3: [InstSimplify] fold icmp with mul nsw and constant operands.
Aug 6 2020, 1:20 PM
bjope added inline comments to rGe8760bb9a8a3: [InstSimplify] fold icmp with mul nsw and constant operands.
Aug 6 2020, 12:18 PM
bjope added inline comments to D85394: [NewPM][GuardWidening] Fix loop guard widening tests under NPM.
Aug 6 2020, 12:33 AM · Restricted Project

Aug 5 2020

bjope added a comment to D83216: [Intrinsic] Add sshl.sat/ushl.sat, saturated shift intrinsics..

LGTM, but i would have preferred to see more feedback on the RFC thread.
That being said, unless you are planning on forming calls to these intrinsics in middle-end transform passes
(i.e. only planning on using them in clang codegen), i think this is okay to proceed.

At least for now, I'm not planning to emit them in middleend. I wanted to get most of the basic support in Clang and LLVM before looking at optimization.

However, based on the discussions in D82663, I am planning on sending an RFC for moving the fixed-point semantics and value class to LLVM, and adding a Builder class similar to the MatrixBuilder.

Aug 5 2020, 3:04 PM · Restricted Project
bjope added a comment to D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder..

This is not ideal, since it makes the calculations char size dependent, and breaks for sizes that are not a multiple of the char size.

How can we have a non-bitfield member whose size is not a multiple of the size of a char?

Downstream, we have fixed-point types that are 24 bits large, but where the char size is 16. The type then takes up 2 chars, where 8 of the bits are padding. The only way in Clang to express that the width of the bit representation of a type should be smaller than the number of chars it takes up in memory -- and consequently, produce an i24 in IR -- is to return a non-charsize multiple from getTypeInfo.

This violates the C and C++ language rules, which require the size of every type to be a multiple of the size of char. A type with 24 value bits and 8 padding bits should report a type size of 32 bits, just like bool reports a size of CHAR_BIT bits despite having only 1 value bit, and x86_64 long double reports a type size of 128 bits despite having only 80 value bits.

Aug 5 2020, 1:09 PM · Restricted Project

Aug 4 2020

bjope added a comment to D82998: [BasicAA] Enable -basic-aa-recphi by default.

Thanks for the fixes Dave. Will make sure that we start using this in our (downstream) fuzzy testing again.

Aug 4 2020, 3:51 AM · Restricted Project

Aug 3 2020

bjope added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Aug 3 2020, 11:41 PM · Restricted Project
bjope added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Aug 3 2020, 3:49 PM · Restricted Project
bjope added inline comments to D85157: [Sema] Add casting check for fixed to fixed point conversions.
Aug 3 2020, 3:39 PM · Restricted Project

Jul 31 2020

bjope committed rG0d25d3b7e3e3: [clang-tidy] Fix build problem after commit 45a720a864320bbbeb596a (authored by bjope).
[clang-tidy] Fix build problem after commit 45a720a864320bbbeb596a
Jul 31 2020, 5:31 AM

Jul 29 2020

bjope accepted D80274: [MachineVerifier] Handle the PHI node for verifyLiveVariables().

LGTM!

Jul 29 2020, 5:59 AM · Restricted Project