00:08airlied: karolherbst, jenatali : is CL meant to be robust wrt to oob buffer access?
00:08jenatali: airlied: Buffers, no
00:08jenatali: Images, yes
00:09jenatali: airlied: https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#operators-indirection - If an invalid value has been assigned to the pointer, the behavior of the unary * operator is undefined .
00:09airlied: piglit bswap test is rogue and doing an oob access, which it gets awya with sometimes, was confusing CI
00:15karolherbst: airlied: check the sampler stuff for images
00:16karolherbst: spolier: CL allows you to specify OOB behaviour
00:18jenatali: karolherbst: Yeah, and all but one of them are well-defined
00:18karolherbst: huh? they are all well defined, no?
00:18jenatali: There's one which isn't
00:19karolherbst: ohh, I thought you meant one is well defined, the others are not :D
00:19karolherbst: well.. CLK_ADDRESS_NONE is undefined obbviously :p
00:19jenatali: That's all I meant
00:20karolherbst: at least they are easy to lower :/
00:26jenatali: jekstrand, karolherbst: Just submitted a set of CL conformance logs \o/
00:31airlied: jenatali: nice
00:56airlied:gotta fix some memory accesses in llvmpipe to stabilise ci I guess
01:01airlied: the test is broken in one case, does oob read/write, but the other case the driver was
01:30airlied:wonders why I'm getting no req local mem size for tests which use shared
01:31jekstrand: input.cl:4:14: error: implicit declaration of function 'convert_float_sat' is invalid in OpenCL
01:31jekstrand: That's... unexpected
01:31airlied: local arrays don't seem to be included in shared mem size
01:31airlied: I wonder where clover execpts me to get sizes from
01:31airlied: karolherbst: ?
01:31jekstrand: Wait, that one's not valid or at least pointless.
01:36airlied: ah I need to use the nir shared_size
01:37jenatali: jekstrand: That sounds like an artifact of karolherbst's runner
01:43jekstrand: jenatali: Yeah, it does
02:46airlied: okay two more fixes needed for llvmpipe and CI looks more stable
03:26jekstrand: airlied: \o/
03:44jekstrand: jenatali, karolherbst: Conversions appear to work. I've got 10 failing tests but they all appear to be bugs in our int64/uint64 -> float lowering code which has nothing to do with the CL-style conversion lowering. I checked one of the failing tests and it passes on Skylake (which has real int64 hardware). I'm doing a full (weak) run on Skylake just to be sure.
03:57airlied: jekstrand: does that just leave printf? :-P
04:02dcbaker[m]: Isn't that optional now? :)
04:24jenatali: jekstrand: Awesome :) I thought Boris's int64 -> float lowering worked, at least after https://gitlab.freedesktop.org/kusma/mesa/-/commit/9b04c8af6d4bb6b6b689fe803ab58c0904fd2677, which I thought was upstream...
04:25jenatali: Yeah, looks like it should be: https://gitlab.freedesktop.org/mesa/mesa/-/commit/199bea0fd80e65178a9d12c705a9f0aaf0a36ceb
04:49jenatali: airlied: Did you see that the libclc change got its own Phoronix article? :P
04:52airlied: I'm just waiting for my renaming vallium article :-P
05:02airlied: jenatali, jenatali : what's the scratch space scope?
05:03airlied:probably needs to consider how to malloc things for t
07:15danvet_: mlankhorst_, airlied I have a bunch of important-ish fixes in drm-misc-fixes
07:15danvet_: well console font out-of-bounds fix, fbdev regression revert + one doc fix :-)
07:16mlankhorst_: one pul request coming up then
07:27airlied: I'll send another or update to Linus, we have another week anyways
07:36danvet_: airlied, looking at your vmwgfx fix, are patches to move ttm_resource_manager->use_type into vmwgfx in flight somewhere?
07:36danvet_: just stumbled over that again
07:47mlankhorst_: oo, evil
08:04danvet_: mlankhorst_, ?
08:06mlankhorst_: suspending ttm entirely
08:11MrCooper: yshui`: "the pending swap has to finish, before the comnands can be processed" assumes the GLX implementation only uses 2 buffers, but it can use more (and Mesa does with DRI3)
08:25yshui`: MrCooper more buffers = more latency
08:26MrCooper: right, just saying you cannot rely on that, you have to explicitly wait for the swap to finish
10:34tarceri: mareko: Sorry for the delay. I believe the correct fix is simply https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6958/
14:01jekstrand: jenatali: You're sure they all passed before? Somethings wrong with int->float and it's not just the SW impl.
14:02jenatali: jekstrand: I have 100% on NVIDIA, two failures on WARP, and a handful on Intel
14:10jenatali: airlied: Scratch space is per-thread
14:21jekstrand: jenatali: It's possible there's something funny going on with our hardware
14:21jekstrand: jenatali: The case is ulong->float
14:24jenatali: jekstrand: Yeah, I see some int -> float failures as well on my Intel device. I think I have a Kabylake
14:25jekstrand: jenatali: Do you know which ones off-hand?
14:26jenatali: jekstrand: convert_floatn_rtp( uintn ), and rtn and rtz, and for int
14:27jenatali: jekstrand: And also all long/ulong -> float conversions
14:28jenatali: jekstrand: All of those cases pass on NVIDIA and WARP though (WARP has some rte failures for int/uint -> float)
14:32jekstrand: jenatali: Ok, it's probably just my hardware being weird then
14:32jekstrand: jenatali: Here's my fail list:
14:32jekstrand: *** 765) convert_ulongn_sat( floatn ) FAILED **
14:32jekstrand: *** 766) convert_ulongn_sat_rte( floatn ) FAILED **
14:32jekstrand: *** 767) convert_ulongn_sat_rtp( floatn ) FAILED **
14:32jekstrand: *** 768) convert_ulongn_sat_rtn( floatn ) FAILED **
14:32jekstrand: *** 769) convert_ulongn_sat_rtz( floatn ) FAILED **
14:32jekstrand: *** 865) convert_longn_sat( floatn ) FAILED **
14:32jekstrand: *** 866) convert_longn_sat_rte( floatn ) FAILED **
14:32jekstrand: *** 867) convert_longn_sat_rtp( floatn ) FAILED **
14:32jekstrand: *** 868) convert_longn_sat_rtn( floatn ) FAILED **
14:32jekstrand: *** 869) convert_longn_sat_rtz( floatn ) FAILED **
14:32jekstrand:runs without -w just for grins
14:33jekstrand: Or frowns as the case may be. :)
14:33jenatali: jekstrand: You'll get results tomorrow :)
14:33jekstrand: jenatali: That's ok. It's on one of my test machines. The only real downside is fan noise
14:38jenatali: jekstrand: Thanks again for helping out with that conversion intrinsic. Looks great, and that was wayyy faster than I could've done it :)
14:38jenatali: Pretty exciting how real CL is getting for Clover
14:43jekstrand: karolherbst: Did you have any more opinions on conversion?
14:43karolherbst: jekstrand: well. Just that I'd like to only have to care about one interfaces really.. so either alu ops or intrinsics. Dealing with both sounds kind of annoying
14:52jekstrand: karolherbst: We can have a pass which turns ALU ops into intrinsics if you really care
14:53jekstrand: And you can run that super-late
14:53karolherbst: yeah, I guess that would work
14:53jenatali: That seems reasonable to me, yeah
14:53jekstrand: It'd probably be less code to just handle it in your back-end. :P
14:54jekstrand: But the NIR pass is probably 20 lines of code. I can type that.
14:54jenatali: It's just a nir_shader_lower_instructions call
15:01karolherbst: jekstrand: yeah.. I don't have a strong opinion on that, I just like a bit of consistency and it would be less code overall
15:01karolherbst: at least for codegen
15:05jenatali: jekstrand: Do you have opinions on what should happen with the f2f16 rounding opcodes? We need the other two rounding modes for CL, but it's not clear to me if we should add opcodes for that, or what
15:07jekstrand: jenatali: I'm kind-of inclined to delete the ones we have
15:08karolherbst: yeah.. I think with the lowering pass to the intrinsics there is really no need for them anymore?
15:08karolherbst: but it just sounds easier to consume the intrinsic in backends
15:09jekstrand: jenatali: I've left them in for now but now that we have the full thing, my inclination is to implement it in the back-end for a few conditions and then get rid if the two ops we have today.
15:09jenatali: jekstrand: Yeah that makes sense
15:09jenatali: I think the only thing left (besides printf) is the vload_half/vstore_half then, which touches the f2f16 + rounding
15:09jekstrand: karolherbst: There you go. Wrote you some (completely untested) code. Check the MR.
15:10jekstrand: jenatali: Sure but that should be easy.
15:10karolherbst: cool. Will do
15:10jekstrand: jenatali: What test case should I look at?
15:10jenatali: jekstrand: Agreed, especially since I already wrote part of it ;)
15:10jekstrand: jenatali: I assume it's just an extra case on vload/vstore
15:10jenatali: jekstrand: test_half
15:10jenatali: jekstrand: Feel free to cannibalize as much (or as little) of https://gitlab.freedesktop.org/kusma/mesa/-/merge_requests/255 as you want
15:12karolherbst: jekstrand: did you push it already?
15:13jekstrand: karolherbst: yup
15:13karolherbst: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6945 right?
15:13karolherbst: because I can't find it
15:13jekstrand: karolherbst: Pushed the wrong branch
15:14karolherbst: now it's there :) thanks
15:14karolherbst: oh wow.. that is indeed trivial
15:14jenatali: Heh, you were right on the mark with ~20 lines
16:28AndrewR: airlied, there is like 558 crashes in piglit cl over llvmpipe (with many tests failing with "-11" ). Is this normal at this stage?
16:45jekstrand: AndrewR: Yes
16:46jekstrand: AndrewR: I don't know what the exact number should be but the SPIR-V clover path isn't very mature.
16:56jekstrand: mareko: Your fp16 patch busted all CPUs HSW and older
17:01jenatali: jekstrand: Looks like 2020f213 ended up still authored by me, but you added my r-b to it. Probably want to either overwrite the author (r-b still stands, the cherry-pick + fixups look good) or add your own r-b
17:02jekstrand: jenatali: How about I add both RBs. That way it's clear that I read your original code and you read my changes.
17:02jenatali: jekstrand: Works for me
17:02jekstrand: I'm trying to keep authorship as much as I can
17:02jekstrand: But there's so much reworking going on
17:02jenatali: Yeah, makes sense
17:03jekstrand: jenatali: I did add one more fixup patch. Not sure if you saw
17:03jenatali: jekstrand: I'm looking at it now
17:04jekstrand: My test run is almost done. I'm on convert_long_sat_rtn( ulong
17:04jenatali: The full test run?
17:05jenatali: Ah ok, that makes more sense :)
17:05jekstrand: FAILED 10 of 765 sub-tests.
17:05jekstrand: That looks consistent for ICL
17:05jekstrand: the 10 are int64 tests
17:06jenatali: jekstrand: Is Intel using bbrezillon's int64 -> float lowering? Or is there a driver-specific lowering?
17:06jekstrand: The full test run I started on my test machine earlier this morning is on convert_char_sat_rtp( uint ) :-)
17:06jekstrand: jenatali: We should be using bbrezillon's lowering but it got a bunch of changes during review. It might have gotten broken. :-/
17:07Venemo: hey jekstrand can I ask yourself or Kayden to review the first 3 commits here: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6964 they are the nir_lower_gs_intrinsics features that we talked about a while ago.
17:07jenatali: jekstrand: You can compare it to what's in our CLOn12 tree, that works - it should be the same though
17:07jekstrand: It's also possible that we have some other int->float cases wrong in hardware and it depends on that behavior.
17:07jenatali: That seems more likely based on the results I was seeing on Intel
17:12jenatali: jekstrand: You might want to add https://gitlab.freedesktop.org/kusma/mesa/-/merge_requests/311/diffs?commit_id=46d808d3dce67e24950f7486112ed12a66a551c0 onto the half patch
17:13jekstrand: jenatali: Will do
17:35jekstrand: jenatali: Have you seen this: https://paste.centos.org/view/a9219ada
17:35jekstrand: jenatali: Looks like a SPIRV-LLVM-Translator bug
17:36jekstrand: Or a clang bug
17:36jenatali: Oh is that coming from the validator?
17:36jekstrand: Yeah. Actually, that looks like a validator bug
17:36jenatali: I haven't/don't run the SPIR-V validator
17:36jenatali: No, that looks like correct validation
17:37jenatali: There's separate opcodes for scalar (vloada_half) vs vector (vloada_halfn)
17:37jenatali: So, the n for the vector version shouldn't be 1
17:37jekstrand: Yeah, I see that in the SPIR-V spec now.
17:37jekstrand: So that's a translator bug
17:37jenatali: Yeah, probably
17:37jenatali: FWIW the vtn code handles it just fine
17:37jekstrand: Our impl doesn't care
17:38jekstrand: In any case, modulo that bug, I'm passing all the half tests with -w
17:43jenatali: jekstrand: The aligned version doesn't have a non-vector version
17:43jenatali: jekstrand: So I think it might be a validator bug
17:44jekstrand: jenatali: No, it's still a translator bug. It should use the normal version for scalars
17:44jekstrand: Because vector alignment is pointless
17:44jenatali: Oh, sure
17:45jenatali: They even go out of their way to force the vector version for aligned load/store of vec1...
17:48jenatali: jekstrand: Are you filing an issue on the translator? Or should I?
17:49jekstrand: jenatali: I can
17:53jekstrand: jenatali: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/765
17:54jekstrand: jenatali: Feel free to comment if you wish
17:54jenatali: jekstrand: Awesome, thanks :)
17:54jekstrand: I don't feel that inclined to write a patch for that today
17:54karolherbst: jekstrand: you probably have to write the patch as well :p
17:57jekstrand: karolherbst: Our processor can consume it. Do I really have to care?
17:57jekstrand: jenatali: You said they had a special case? Where do I find it?
17:57karolherbst: jekstrand: I guess not :D
17:57jenatali: jekstrand: I closed that tab, one sec
17:58danvet_: sravn, pinchartl did you see the bridge revert to reinstante the connector?
17:58jenatali: jekstrand: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/a88d869eee6ad76b9ce89d791ae2cf340459a564/lib/SPIRV/OCL20ToSPIRV.cpp#L1388
17:58danvet_: would be good to take a look and ack that you're aware
17:58jenatali: The 'true' forces using the vector version
17:58danvet_: maybe even hash out some path forward
18:04sravn: danvet_: Saw it the other day, missing time to focus on it. Maybe tomorrow. I have some days/weeks ago asked for some feedback, if I got it then it is lost while I was (and is) almost offline
18:12airlied: AndrewR: seems high
18:19airlied: AndrewR: CI was seeing "[709/709] skip: 74, pass: 293, fail: 318, crash: 24"
18:19airlied: with the fixes I've just queued up for marge now
18:23jekstrand: karolherbst: To my brain (which doesn't understand clover well!), !6569 most looks good. I left a few comments but otherwise, I think we can probably land soon.
18:24karolherbst: yeah, I already saw your comments :D
18:28airlied: is that the MR that fixes my load_constant_ptr ?
18:30jekstrand: That's gonna get a lot more tests passing
18:47airlied: yeah that + scratch seems to be the largest current crashes
19:04jekstrand: jenatali: And now I've filed a spec issue.... It's turtles all the way down, I'm afraid.
19:05jenatali: jekstrand: Oh no, for what?
19:05jekstrand: vloada_half :)
19:05jekstrand: It's very unclear in the spec whether or not it exists
19:05jekstrand: CL 1.0 says no, CL 1.1 arguably says yes and CL 1.2 arguably says no again.
19:06jekstrand: And the CTS says yes for CL 1.1+, I think.
19:06jenatali: Looks like the CTS says yes for CL 1.0 and 1.2
19:06jenatali: Though I guess the CTS doesn't really have 1.0 or 1.1 support...
19:06jekstrand: jenatali: There are too many negatives there
19:07jekstrand: It says no for !defined(CL_VERSION_1_1)
19:07jekstrand: so yes for CL_VERSION_1_1 which is defined for all 1.1+ versions
19:07jekstrand: so it says it got added in 1.1
19:07jenatali: Oh, you're right
19:09jekstrand: Someone should just throw a static inline in the opencl-c.h which makes vloada_half() call vload_half() and call it a day.
19:09jekstrand: It's definitely in the header though
19:10jenatali: Heh, that's a good idea
19:12jenatali: jekstrand: Any reason you didn't use the public specs repo? https://github.com/KhronosGroup/OpenCL-Docs/issues
19:14jekstrand: jenatali: internal is my default?
19:14jekstrand: Sorry. Habbit.
19:14jekstrand: I probably could have used the public one
19:14jenatali: Heh, no need to apologize, was just curious
19:14jekstrand:spends way too much time on the Khronos gitlab
19:28mareko: jekstrand: I'm aware, solution 1 is to switch to inline assembly, solution 2 is to disable -mf16c before we switch to inline assembly
19:29mareko: solution 3 is to use -mf16c on gcc versions that are not broken
19:30jekstrand: mareko: Yeah, I don't know what the solution is. I was just making sure you're aware of it. It sounds like others found out long before me though.
19:52Venemo: what is the problem?
20:04HdkR: Venemo: I'm going to guess the f16c problem
20:05jekstrand: Yup, that
20:11HdkR: It's a shame that gcc function multiversioning isn't worth using
20:14vsyrjala: iirc in igt we use ifunc + some pgrama magic for f16c
20:15anholt: an ifunc actually sounds reasonable for us, I think we wouldn't run into any of ifunc's many traps.
20:17HdkR: One of the complaints was performance hit when it is a function call for conversion for the NV_half_float extension? Would want to replace entire functions using the conversion ops?
20:28robclark: mattst88, mareko: fwiw that f16c thing also is breaking building for arm with clang..
20:29robclark: maybe we can just revert and try again?
20:29mattst88: oh, that's ... strange
20:30mattst88: robclark: does https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6951 fix it on arm?
20:31robclark: I'm looking at !6951 .. I'm not sure it actually fixes it? Hmm, or if it removes the immintrin.h include it might..
20:32robclark: give me a min, I can try it locally and see.. just in middle of other debug..
20:34robclark: mattst88: no, same issue still, I think the problem is it decides the compiler supports f16c.. but not sure if the intrinsics/header are intended to be cross-platform
20:34robclark: ie. vs using some neon intrinsics
20:34robclark:hasn't had a chance to look closely yet, so just hacked around it for now
20:35anholt: feels like time to back the original MR out
20:35HdkR: fun fact. AArch64 has f16<->f32 conversion, but to get the different rounding modes you have to modify FPSR
20:35mattst88: weird, for that to break on ARM... clang has to claim to support -mf16c on arm.
20:36anholt: clang: warning: argument unused during compilation: '-mf16c' [-Wunused-command-line-argument]
20:36anholt: thanks clang
20:36anholt: (that was me just invoking the android ndk compiler on an empty .c file)
20:37mattst88: dcbaker: how are we supposed to use cc.has_argument() if clang does that?
20:37HdkR: Same on a local ARMv8 Linux clang
20:37robclark: so.. TIL that a single clang binary can support multiple arch's.. which might be part of th eproblem
20:38mattst88: I'm personally fine with reverting the original patch or committing !6951, but obviously !6951 won't fix the issue on ARM without additional work
20:39robclark: we could also move the f16c check inside of the if-x86 part in meson.build..
20:39robclark: but maybe we should revert first, and then figure out better soln
20:39dcbaker[m]: mattst88: .... clang renamed the warning
20:39mattst88: yeah, I guess it would be easy enough to add an if host_machine.cpu_family().startswith('x86') to the check
20:40dcbaker[m]: or we (meson) added the wrong error to clang
20:40mattst88: dcbaker[m]: oh shit..
20:41dcbaker[m]: nope, we just never added the error at all
20:41dcbaker[m]: I know we have unit tests for this...
20:41dcbaker[m]: how the hell are we passing them?
20:42mattst88: robclark, anholt: the MR that added the F16C code passed all the CI pipelines (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6774). are we missing a configuration that would have caught this?
20:43dcbaker[m]: no, we have the right ones, at least on master
20:43dcbaker[m]: what version are you using?
20:44robclark: mattst88: I think it is gcc vs clang
20:44dcbaker[m]: errrr, that's on a dev branch
20:44robclark: I also didn't notice the error until I tried to uprev cros
20:44robclark: (since for dev on linux I use gcc)
20:44mattst88: makes sense. I guess we need some clang CI pipelines
20:44robclark: yeah, clang cross compile would be useful
20:45dcbaker[m]: I see meson injecting: -Werror=unknown-warning-option -Werror=unused-command-line-argument -Werror=ignored-optimization-argument for clang has_argument
20:46mattst88: so, mareko didn't seem thrilled by my patch since it presumably doesn't offer the same performance as he was aiming for
20:46mattst88: I am going to try another approach using inline assembly, but I think we should probably revert the original patches first
20:46mattst88: anyone excited to put together a revert MR, or want me to?
20:47robclark: +1 for revert for now, and go for it if you have time, I haven't had a chance yet
20:47mattst88: sure, will do
21:39karolherbst: airlied: by how much does https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6569 improve llvmpips CL pass rate?
21:55mattst88: I'm guessing that https://gitlab.freedesktop.org/mesa/mesa/-/jobs/4794202 is dead?
21:55mattst88: Runner: google-freedreno-cheza-7
22:01anholt: yeah, but we're supposed to be catching "POWER_GOOD is not asserted"
22:02anholt: sorry, "not seen in time" above that
22:07anholt: mattst88: I'm reviewing cros_servo_run.py, and not seeing how it could have not matched that line
22:13anholt: oh. wait, that's obvious now.
22:14anholt: got to put the check in the loop before the CPU comes up in the bootloader, not the loop after when we're waiting for the tests to run.
22:15dcbaker[m]: mattst88: I'm very sure that meson does the correct injection for clang to make has_argument work. It doesn't (I already knew) do it correctly for some compilers, but GCC and Clang should work
22:16mattst88: anholt: ah, cool. thanks for taking a look
22:16mattst88: dcbaker[m]: bizarre..
22:16dcbaker[m]: what version of meson do you have?
22:16mattst88: dcbaker[m]: what flag does clang need to error out on unknown flags?
22:17dcbaker[m]: -Werror=unknown-warning-option -Werror=unused-command-line-argument -Werror=ignored-optimization-argument
22:17mattst88: I haven't reproduced the issue robclark saw, so my meson version probably isn't relevant, but I have 0.54.3
22:17dcbaker[m]: at least, that's what meson master feeds clang
22:30anholt: dcbaker[m]: adding those args turns it from warn to fail on the android compiler, at least.
22:31dcbaker[m]: It may be an issue where an older version of meson didn't have those, or they weren't applied correctly
22:31dcbaker[m]: I've been doing a *lot* of work on the compiler stuff in meson lately and finding all the bugs
23:20airlied: karolherbst: https://paste.centos.org/view/7b8043d4
23:20airlied: before after/numbers
23:21airlied:is off today, so going out :-P
23:28AndrewR: karolherbst, airlied : with !6040 applied it seems to go higher? [710/710] skip: 73, pass: 495, fail: 24, timeout: 1, crash: 117
23:28karolherbst: ahh printf..
23:29jenatali: Huh, that's not the nir printf though
23:30FLHerne: jekstrand: There's a typo in cb95065d , 'covnersion' isn't a word ;-)
23:31FLHerne: in `bool nir_lower_alu_covnersion_to_intrinsic(nir_shader *shader);`
23:35jekstrand: FLHerne: Bah
23:35jekstrand: FLHerne: You want to send a patch or shall I?
23:36FLHerne: jekstrand: You'd better, I'd have to make an account :p
23:36jekstrand: FLHerne: Fair enough.
23:38karolherbst: FLHerne: "make"? like don't you even have a github account? :p
23:38FLHerne: freedesktop, no?
23:39FLHerne: I forgot, I did register one to file a bug against libinput
23:39FLHerne: But nvm
23:40jekstrand: FLHerne: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6975
23:41FLHerne: Smallest MR ever? :p
23:41jekstrand: No, it's 2 lines. :)
23:41FLHerne: That's the sort of thing where in kdevelop I'd not bother and just push directly (and then push the wrong branch and have everyone yell at me...)
23:41FLHerne: By character count, maybe
23:42jekstrand: FLHerne: I assigned it to Marge when I made the MR. I'm not going to bother waiting for a review.
23:42jekstrand: The only reason I didn't just push is because people get grumpy when Marge's queue gets messed up.
23:42jekstrand: MrCooper, you're welcome. :-P
23:46jenatali: Sorry I missed it, that's the kind of thing review's supposed to catch
23:46karolherbst: FLHerne: I really meant github. We do have social logins on the fdo gitlab instance
23:46karolherbst: so you can just login with github